Hi,
Changelog
v2:
Patch nand_lock() & nand_unlock() for MTD->_lock/_unlock default call-back
function replacement.
Patch nand_suspend() & nand_resume() with manufacturer specific operation.
v1:
Patch manufacturer post_init for MTD->_lock/_unlock & MTD->_suspend/_resume
replacement.
thanks for your time & review.
Mason
Mason Yang (4):
mtd: rawnand: Add support manufacturer specific lock/unlock operatoin
mtd: rawnand: Add support Macronix Block Protection function
mtd: rawnand: Add support manufacturer specific suspend/resume
operation
mtd: rawnand: Add support Macronix deep power down mode
drivers/mtd/nand/raw/nand_base.c | 41 ++++++++++-
drivers/mtd/nand/raw/nand_macronix.c | 137 ++++++++++++++++++++++++++++++++++-
include/linux/mtd/rawnand.h | 5 ++
3 files changed, 175 insertions(+), 8 deletions(-)
--
1.9.1
Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
operation while the device supports Block Protection function.
Signed-off-by: Mason Yang <[email protected]>
---
drivers/mtd/nand/raw/nand_base.c | 32 ++++++++++++++++++++++++++++++--
include/linux/mtd/rawnand.h | 3 +++
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 5c2c30a..5e318ff 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4356,6 +4356,34 @@ static void nand_shutdown(struct mtd_info *mtd)
nand_suspend(mtd);
}
+/**
+ * nand_lock - [MTD Interface] Lock the NAND flash
+ * @mtd: MTD device structure
+ */
+static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+
+ if (!chip->_lock)
+ return -ENOTSUPP;
+
+ return chip->_lock(chip, ofs, len);
+}
+
+/**
+ * nand_unlock - [MTD Interface] Unlock the NAND flash
+ * @mtd: MTD device structure
+ */
+static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+
+ if (!chip->_unlock)
+ return -ENOTSUPP;
+
+ return chip->_unlock(chip, ofs, len);
+}
+
/* Set default functions */
static void nand_set_defaults(struct nand_chip *chip)
{
@@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip)
mtd->_read_oob = nand_read_oob;
mtd->_write_oob = nand_write_oob;
mtd->_sync = nand_sync;
- mtd->_lock = NULL;
- mtd->_unlock = NULL;
+ mtd->_lock = nand_lock;
+ mtd->_unlock = nand_unlock;
mtd->_suspend = nand_suspend;
mtd->_resume = nand_resume;
mtd->_reboot = nand_shutdown;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 4ab9bcc..2430ecd 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1136,6 +1136,9 @@ struct nand_chip {
const struct nand_manufacturer *desc;
void *priv;
} manufacturer;
+
+ int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
+ int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
};
extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
--
1.9.1
Macronix AC series support using SET_FEATURES to change
Block Protection and Unprotection.
Signed-off-by: Mason Yang <[email protected]>
---
drivers/mtd/nand/raw/nand_macronix.c | 69 +++++++++++++++++++++++++++++++++---
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 58511ae..13929bf 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -11,6 +11,10 @@
#define MACRONIX_READ_RETRY_BIT BIT(0)
#define MACRONIX_NUM_READ_RETRY_MODES 6
+#define ONFI_FEATURE_ADDR_MXIC_PROTECTION 0xA0
+#define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
+#define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
+
struct nand_onfi_vendor_macronix {
u8 reserved;
u8 reliability_func;
@@ -57,7 +61,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
* the timings unlike what is declared in the parameter page. Unflag
* this feature to avoid unnecessary downturns.
*/
-static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
+static int macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
{
unsigned int i;
static const char * const broken_get_timings[] = {
@@ -78,7 +82,7 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
};
if (!chip->parameters.supports_set_get_features)
- return;
+ return 0;
for (i = 0; i < ARRAY_SIZE(broken_get_timings); i++) {
if (!strcmp(broken_get_timings[i], chip->parameters.model))
@@ -86,22 +90,79 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
}
if (i == ARRAY_SIZE(broken_get_timings))
- return;
+ return 0;
bitmap_clear(chip->parameters.get_feature_list,
ONFI_FEATURE_ADDR_TIMING_MODE, 1);
bitmap_clear(chip->parameters.set_feature_list,
ONFI_FEATURE_ADDR_TIMING_MODE, 1);
+ return 1;
+}
+
+/*
+ * Macronix NAND supports Block Protection by Protectoin(PT) pin;
+ * active high at power-on which protects the entire chip even the #WP is
+ * disabled. Lock/unlock protection area can be partition according to
+ * protection bits, i.e. upper 1/2 locked, upper 1/4 locked and so on.
+ */
+static int mxic_nand_lock(struct nand_chip *chip, loff_t ofs, uint64_t len)
+{
+ u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+ int ret;
+
+ feature[0] = MXIC_BLOCK_PROTECTION_ALL_LOCK;
+ nand_select_target(chip, 0);
+ ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+ feature);
+ nand_deselect_target(chip);
+ if (ret)
+ pr_err("%s all blocks failed\n", __func__);
+
+ return ret;
+}
+
+static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
+{
+ u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+ int ret;
+
+ feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
+ nand_select_target(chip, 0);
+ ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+ feature);
+ nand_deselect_target(chip);
+ if (ret)
+ pr_err("%s all blocks failed\n", __func__);
+
+ return ret;
}
+/*
+ * Macronix NAND AC series support Block Protection by SET_FEATURES
+ * to lock/unlock blocks.
+ */
static int macronix_nand_init(struct nand_chip *chip)
{
+ bool blockprotected = false;
+
if (nand_is_slc(chip))
chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
- macronix_nand_fix_broken_get_timings(chip);
+ if (macronix_nand_fix_broken_get_timings(chip))
+ blockprotected = true;
+
macronix_nand_onfi_init(chip);
+ if (blockprotected) {
+ bitmap_set(chip->parameters.set_feature_list,
+ ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+ bitmap_set(chip->parameters.get_feature_list,
+ ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+
+ chip->_lock = mxic_nand_lock;
+ chip->_unlock = mxic_nand_unlock;
+ }
+
return 0;
}
--
1.9.1
Macronix AD series support deep power down mode for a minimum
power consumption state.
Patch nand_suspend() & nand_resume() by Macronix specific
deep power down mode command and exit it.
Signed-off-by: Mason Yang <[email protected]>
---
drivers/mtd/nand/raw/nand_macronix.c | 72 +++++++++++++++++++++++++++++++++++-
1 file changed, 70 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 13929bf..3098bc0 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -15,6 +15,8 @@
#define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
#define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
+#define NAND_CMD_POWER_DOWN 0xB9
+
struct nand_onfi_vendor_macronix {
u8 reserved;
u8 reliability_func;
@@ -137,13 +139,66 @@ static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
return ret;
}
+int nand_power_down_op(struct nand_chip *chip)
+{
+ int ret;
+
+ if (nand_has_exec_op(chip)) {
+ struct nand_op_instr instrs[] = {
+ NAND_OP_CMD(NAND_CMD_POWER_DOWN, 0),
+ };
+
+ struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
+
+ ret = nand_exec_op(chip, &op);
+ if (ret)
+ return ret;
+
+ } else {
+ chip->legacy.cmdfunc(chip, NAND_CMD_POWER_DOWN, -1, -1);
+ }
+
+ return 0;
+}
+
+static int mxic_nand_suspend(struct nand_chip *chip)
+{
+ int ret;
+
+ nand_select_target(chip, 0);
+ ret = nand_power_down_op(chip);
+ if (ret < 0)
+ pr_err("%s called for chip into suspend failed\n", __func__);
+ nand_deselect_target(chip);
+
+ return ret;
+}
+
+static void mxic_nand_resume(struct nand_chip *chip)
+{
+ /*
+ * Toggle #CS pin to resume NAND device and don't care
+ * of the others CLE, #WE, #RE pins status.
+ * Here sending power down command to toggle #CS line.
+ */
+ nand_select_target(chip, 0);
+ nand_power_down_op(chip);
+ nand_deselect_target(chip);
+}
+
/*
- * Macronix NAND AC series support Block Protection by SET_FEATURES
+ * Macronix NAND AC & AD series support Block Protection by SET_FEATURES
* to lock/unlock blocks.
*/
static int macronix_nand_init(struct nand_chip *chip)
{
- bool blockprotected = false;
+ unsigned int i;
+ bool blockprotected = false, powerdown = false;
+ static const char * const power_down_dev[] = {
+ "MX30LF1G28AD",
+ "MX30LF2G28AD",
+ "MX30LF4G28AD",
+ };
if (nand_is_slc(chip))
chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
@@ -153,6 +208,14 @@ static int macronix_nand_init(struct nand_chip *chip)
macronix_nand_onfi_init(chip);
+ for (i = 0; i < ARRAY_SIZE(power_down_dev); i++) {
+ if (!strcmp(power_down_dev[i], chip->parameters.model)) {
+ blockprotected = true;
+ powerdown = true;
+ break;
+ }
+ }
+
if (blockprotected) {
bitmap_set(chip->parameters.set_feature_list,
ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
@@ -163,6 +226,11 @@ static int macronix_nand_init(struct nand_chip *chip)
chip->_unlock = mxic_nand_unlock;
}
+ if (powerdown) {
+ chip->_suspend = mxic_nand_suspend;
+ chip->_resume = mxic_nand_resume;
+ }
+
return 0;
}
--
1.9.1
Hi Mason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc5 next-20191029]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-rawnand-Add-support-Macronix-Block-Protection-deep-power-down-mode/20191030-071757
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 23fdb198ae81f47a574296dab5167c5e136a02ba
reproduce: make htmldocs
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
Warning: The Sphinx 'sphinx_rtd_theme' HTML theme was not found. Make sure you have the theme installed to produce pretty HTML output. Falling back to the default theme.
WARNING: dot(1) not found, for better output quality install graphviz from http://www.graphviz.org
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints'
include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops'
include/linux/w1.h:277: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
include/linux/spi/spi.h:190: warning: Function parameter or member 'driver_override' not described in 'spi_device'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
include/linux/lsm_hooks.h:1822: warning: Function parameter or member 'locked_down' not described in 'security_list_options'
drivers/gpio/gpiolib-of.c:92: warning: Excess function parameter 'dev' description in 'of_gpio_need_valid_mask'
include/linux/i2c.h:337: warning: Function parameter or member 'init_irq' not described in 'i2c_client'
lib/genalloc.c:1: warning: 'gen_pool_add_virt' not found
lib/genalloc.c:1: warning: 'gen_pool_alloc' not found
lib/genalloc.c:1: warning: 'gen_pool_free' not found
lib/genalloc.c:1: warning: 'gen_pool_alloc_algo' not found
>> include/linux/mtd/rawnand.h:1143: warning: Function parameter or member '_lock' not described in 'nand_chip'
>> include/linux/mtd/rawnand.h:1143: warning: Function parameter or member '_unlock' not described in 'nand_chip'
>> drivers/mtd/nand/raw/nand_base.c:4365: warning: Function parameter or member 'ofs' not described in 'nand_lock'
>> drivers/mtd/nand/raw/nand_base.c:4365: warning: Function parameter or member 'len' not described in 'nand_lock'
>> drivers/mtd/nand/raw/nand_base.c:4379: warning: Function parameter or member 'ofs' not described in 'nand_unlock'
>> drivers/mtd/nand/raw/nand_base.c:4379: warning: Function parameter or member 'len' not described in 'nand_unlock'
fs/posix_acl.c:647: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
fs/posix_acl.c:647: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
fs/posix_acl.c:647: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found
drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found
drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' not found
Error: Cannot open file drivers/dma-buf/reservation.c
Error: Cannot open file drivers/dma-buf/reservation.c
Error: Cannot open file drivers/dma-buf/reservation.c
Error: Cannot open file include/linux/reservation.h
Error: Cannot open file include/linux/reservation.h
kernel/dma/coherent.c:1: warning: no structured comments found
include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry'
include/linux/skbuff.h:888: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'list' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
include/linux/skbuff.h:888: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
include/net/sock.h:233: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
include/net/sock.h:515: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock'
include/net/sock.h:2450: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE'
include/net/sock.h:2450: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE'
include/net/sock.h:2450: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE'
include/linux/netdevice.h:2053: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
include/linux/netdevice.h:2053: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'
mm/util.c:1: warning: 'get_user_pages_fast' not found
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:335: warning: Excess function parameter 'dev' description in 'amdgpu_gem_prime_export'
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:336: warning: Excess function parameter 'dev' description in 'amdgpu_gem_prime_export'
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:142: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:347: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:348: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:494: warning: Function parameter or member 'start' not described in 'amdgpu_vm_pt_first_dfs'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:821: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1283: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2821: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute'
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:378: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:379: warning: Function parameter or member 'ih' not described in 'amdgpu_irq_dispatch'
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:379: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:1: warning: no structured comments found
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1: warning: no structured comments found
vim +1143 include/linux/mtd/rawnand.h
^1da177e4c3f41 include/linux/mtd/nand.h Linus Torvalds 2005-04-16 @1143
:::::: The code at line 1143 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Mason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc5 next-20191030]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-rawnand-Add-support-Macronix-Block-Protection-deep-power-down-mode/20191030-071757
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 23fdb198ae81f47a574296dab5167c5e136a02ba
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
>> drivers/mtd/nand/raw/nand_macronix.c:142:5: sparse: sparse: symbol 'nand_power_down_op' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Fixes: 9dfaf23e330d ("mtd: rawnand: Add support Macronix deep power down mode")
Signed-off-by: kbuild test robot <[email protected]>
---
nand_macronix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 3098bc09c25c7..adf81482937bc 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -139,7 +139,7 @@ static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
return ret;
}
-int nand_power_down_op(struct nand_chip *chip)
+static int nand_power_down_op(struct nand_chip *chip)
{
int ret;
Hi Mason,
Mason Yang <[email protected]> wrote on Mon, 28 Oct 2019 17:55:25
+0800:
> Macronix AC series support using SET_FEATURES to change
> Block Protection and Unprotection.
>
> Signed-off-by: Mason Yang <[email protected]>
> ---
> drivers/mtd/nand/raw/nand_macronix.c | 69 +++++++++++++++++++++++++++++++++---
> 1 file changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
> index 58511ae..13929bf 100644
> --- a/drivers/mtd/nand/raw/nand_macronix.c
> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> @@ -11,6 +11,10 @@
> #define MACRONIX_READ_RETRY_BIT BIT(0)
> #define MACRONIX_NUM_READ_RETRY_MODES 6
>
> +#define ONFI_FEATURE_ADDR_MXIC_PROTECTION 0xA0
> +#define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
> +#define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
> +
> struct nand_onfi_vendor_macronix {
> u8 reserved;
> u8 reliability_func;
> @@ -57,7 +61,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
> * the timings unlike what is declared in the parameter page. Unflag
> * this feature to avoid unnecessary downturns.
> */
> -static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
> +static int macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
> {
> unsigned int i;
> static const char * const broken_get_timings[] = {
> @@ -78,7 +82,7 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
> };
>
> if (!chip->parameters.supports_set_get_features)
> - return;
> + return 0;
>
> for (i = 0; i < ARRAY_SIZE(broken_get_timings); i++) {
> if (!strcmp(broken_get_timings[i], chip->parameters.model))
> @@ -86,22 +90,79 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
> }
>
> if (i == ARRAY_SIZE(broken_get_timings))
> - return;
> + return 0;
>
> bitmap_clear(chip->parameters.get_feature_list,
> ONFI_FEATURE_ADDR_TIMING_MODE, 1);
> bitmap_clear(chip->parameters.set_feature_list,
> ONFI_FEATURE_ADDR_TIMING_MODE, 1);
> + return 1;
> +}
> +
> +/*
> + * Macronix NAND supports Block Protection by Protectoin(PT) pin;
> + * active high at power-on which protects the entire chip even the #WP is
> + * disabled. Lock/unlock protection area can be partition according to
> + * protection bits, i.e. upper 1/2 locked, upper 1/4 locked and so on.
> + */
> +static int mxic_nand_lock(struct nand_chip *chip, loff_t ofs, uint64_t len)
> +{
> + u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
> + int ret;
> +
> + feature[0] = MXIC_BLOCK_PROTECTION_ALL_LOCK;
> + nand_select_target(chip, 0);
> + ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
> + feature);
> + nand_deselect_target(chip);
> + if (ret)
> + pr_err("%s all blocks failed\n", __func__);
> +
> + return ret;
> +}
> +
> +static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
> +{
> + u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
> + int ret;
> +
> + feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
> + nand_select_target(chip, 0);
> + ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
> + feature);
> + nand_deselect_target(chip);
> + if (ret)
> + pr_err("%s all blocks failed\n", __func__);
> +
> + return ret;
> }
>
> +/*
> + * Macronix NAND AC series support Block Protection by SET_FEATURES
> + * to lock/unlock blocks.
> + */
> static int macronix_nand_init(struct nand_chip *chip)
> {
> + bool blockprotected = false;
> +
> if (nand_is_slc(chip))
> chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
>
> - macronix_nand_fix_broken_get_timings(chip);
> + if (macronix_nand_fix_broken_get_timings(chip))
> + blockprotected = true;
I don't like this at all :)
Please create a helper which detects which part is broken/protected
then create helpers to act in this case.
If the list is absolutely identical, you can share the detection
helper. Otherwise, if you think the list can diverge, please only share
the list for now and create two detection helpers.
> +
> macronix_nand_onfi_init(chip);
>
> + if (blockprotected) {
> + bitmap_set(chip->parameters.set_feature_list,
> + ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
> + bitmap_set(chip->parameters.get_feature_list,
> + ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
> +
> + chip->_lock = mxic_nand_lock;
> + chip->_unlock = mxic_nand_unlock;
> + }
> +
> return 0;
> }
>
Thanks,
Miquèl
Hi Mason,
Mason Yang <[email protected]> wrote on Mon, 28 Oct 2019 17:55:27
+0800:
> Macronix AD series support deep power down mode for a minimum
> power consumption state.
>
> Patch nand_suspend() & nand_resume() by Macronix specific
> deep power down mode command and exit it.
>
> Signed-off-by: Mason Yang <[email protected]>
> ---
> drivers/mtd/nand/raw/nand_macronix.c | 72 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
> index 13929bf..3098bc0 100644
> --- a/drivers/mtd/nand/raw/nand_macronix.c
> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> @@ -15,6 +15,8 @@
> #define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
> #define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
>
> +#define NAND_CMD_POWER_DOWN 0xB9
I suppose this value is Macronix specific, and hence should have a
MACRONIX_ or MXIC_ prefix instead of NAND_.
> +
> struct nand_onfi_vendor_macronix {
> u8 reserved;
> u8 reliability_func;
> @@ -137,13 +139,66 @@ static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
> return ret;
> }
>
> +int nand_power_down_op(struct nand_chip *chip)
> +{
> + int ret;
> +
> + if (nand_has_exec_op(chip)) {
> + struct nand_op_instr instrs[] = {
> + NAND_OP_CMD(NAND_CMD_POWER_DOWN, 0),
> + };
> +
> + struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
> +
> + ret = nand_exec_op(chip, &op);
> + if (ret)
> + return ret;
> +
> + } else {
> + chip->legacy.cmdfunc(chip, NAND_CMD_POWER_DOWN, -1, -1);
> + }
> +
> + return 0;
> +}
> +
> +static int mxic_nand_suspend(struct nand_chip *chip)
> +{
> + int ret;
> +
> + nand_select_target(chip, 0);
> + ret = nand_power_down_op(chip);
> + if (ret < 0)
> + pr_err("%s called for chip into suspend failed\n", __func__);
What about something more specific?
"Suspending MXIC NAND chip failed (%)\n", ret
> + nand_deselect_target(chip);
> +
> + return ret;
> +}
> +
> +static void mxic_nand_resume(struct nand_chip *chip)
> +{
> + /*
> + * Toggle #CS pin to resume NAND device and don't care
> + * of the others CLE, #WE, #RE pins status.
> + * Here sending power down command to toggle #CS line.
The first sentence seems right, the second could be upgraded:
The purpose of doing a power down operation is just to
ensure some bytes will be sent over the NAND bus so that #CS
gets toggled because this is why the chip is woken up.
The content of the bytes sent on the NAND bus are not
relevant at this time. Sending bytes on the bus is mandatory
for a lot of NAND controllers otherwise they are not able to
just assert/de-assert #CS.
> + */
> + nand_select_target(chip, 0);
> + nand_power_down_op(chip);
Are you sure sending a power_down_op will not be interpreted by the
chip?
I would expect a sleeping delay here, even small.
> + nand_deselect_target(chip);
> +}
> +
> /*
> - * Macronix NAND AC series support Block Protection by SET_FEATURES
> + * Macronix NAND AC & AD series support Block Protection by SET_FEATURES
> * to lock/unlock blocks.
> */
> static int macronix_nand_init(struct nand_chip *chip)
> {
> - bool blockprotected = false;
> + unsigned int i;
> + bool blockprotected = false, powerdown = false;
> + static const char * const power_down_dev[] = {
> + "MX30LF1G28AD",
> + "MX30LF2G28AD",
> + "MX30LF4G28AD",
> + };
>
> if (nand_is_slc(chip))
> chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
> @@ -153,6 +208,14 @@ static int macronix_nand_init(struct nand_chip *chip)
>
> macronix_nand_onfi_init(chip);
>
> + for (i = 0; i < ARRAY_SIZE(power_down_dev); i++) {
> + if (!strcmp(power_down_dev[i], chip->parameters.model)) {
> + blockprotected = true;
> + powerdown = true;
> + break;
> + }
> + }
> +
> if (blockprotected) {
> bitmap_set(chip->parameters.set_feature_list,
> ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
> @@ -163,6 +226,11 @@ static int macronix_nand_init(struct nand_chip *chip)
> chip->_unlock = mxic_nand_unlock;
> }
>
> + if (powerdown) {
> + chip->_suspend = mxic_nand_suspend;
> + chip->_resume = mxic_nand_resume;
> + }
See my comment on patch 2.
> +
> return 0;
> }
>
Thanks,
Miquèl
Hi Mason,
Mason Yang <[email protected]> wrote on Mon, 28 Oct 2019 17:55:24
+0800:
> Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
> operation while the device supports Block Protection function.
>
> Signed-off-by: Mason Yang <[email protected]>
> ---
> drivers/mtd/nand/raw/nand_base.c | 32 ++++++++++++++++++++++++++++++--
> include/linux/mtd/rawnand.h | 3 +++
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 5c2c30a..5e318ff 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4356,6 +4356,34 @@ static void nand_shutdown(struct mtd_info *mtd)
> nand_suspend(mtd);
> }
>
> +/**
> + * nand_lock - [MTD Interface] Lock the NAND flash
> + * @mtd: MTD device structure
> + */
> +static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> +
> + if (!chip->_lock)
> + return -ENOTSUPP;
> +
> + return chip->_lock(chip, ofs, len);
> +}
> +
> +/**
> + * nand_unlock - [MTD Interface] Unlock the NAND flash
> + * @mtd: MTD device structure
> + */
> +static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> +
> + if (!chip->_unlock)
> + return -ENOTSUPP;
> +
> + return chip->_unlock(chip, ofs, len);
> +}
> +
> /* Set default functions */
> static void nand_set_defaults(struct nand_chip *chip)
> {
> @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip)
> mtd->_read_oob = nand_read_oob;
> mtd->_write_oob = nand_write_oob;
> mtd->_sync = nand_sync;
> - mtd->_lock = NULL;
> - mtd->_unlock = NULL;
> + mtd->_lock = nand_lock;
> + mtd->_unlock = nand_unlock;
> mtd->_suspend = nand_suspend;
> mtd->_resume = nand_resume;
> mtd->_reboot = nand_shutdown;
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 4ab9bcc..2430ecd 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1136,6 +1136,9 @@ struct nand_chip {
> const struct nand_manufacturer *desc;
> void *priv;
> } manufacturer;
> +
> + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
Kernel documentation is missing here.
Also please fix kbuildtest robot warnings.
With all this done, please add my:
Reviewed-by: Miquel Raynal <[email protected]>
Thanks,
Miquèl
On Mon, 28 Oct 2019 17:55:24 +0800
Mason Yang <[email protected]> wrote:
> /* Set default functions */
> static void nand_set_defaults(struct nand_chip *chip)
> {
> @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip)
> mtd->_read_oob = nand_read_oob;
> mtd->_write_oob = nand_write_oob;
> mtd->_sync = nand_sync;
> - mtd->_lock = NULL;
> - mtd->_unlock = NULL;
> + mtd->_lock = nand_lock;
> + mtd->_unlock = nand_unlock;
> mtd->_suspend = nand_suspend;
> mtd->_resume = nand_resume;
> mtd->_reboot = nand_shutdown;
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 4ab9bcc..2430ecd 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1136,6 +1136,9 @@ struct nand_chip {
> const struct nand_manufacturer *desc;
> void *priv;
> } manufacturer;
> +
> + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
Please drop this _ prefix.
> };
Hi Miquel,
> > /* Set default functions */
> > static void nand_set_defaults(struct nand_chip *chip)
> > {
> > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip
*chip)
> > mtd->_read_oob = nand_read_oob;
> > mtd->_write_oob = nand_write_oob;
> > mtd->_sync = nand_sync;
> > - mtd->_lock = NULL;
> > - mtd->_unlock = NULL;
> > + mtd->_lock = nand_lock;
> > + mtd->_unlock = nand_unlock;
> > mtd->_suspend = nand_suspend;
> > mtd->_resume = nand_resume;
> > mtd->_reboot = nand_shutdown;
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 4ab9bcc..2430ecd 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1136,6 +1136,9 @@ struct nand_chip {
> > const struct nand_manufacturer *desc;
> > void *priv;
> > } manufacturer;
> > +
> > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>
> Kernel documentation is missing here.
>
> Also please fix kbuildtest robot warnings.
okay, will fix both !
>
> With all this done, please add my:
> Reviewed-by: Miquel Raynal <[email protected]>
>
> Thanks,
> Miqu?l
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi Boris,
>
> > /* Set default functions */
> > static void nand_set_defaults(struct nand_chip *chip)
> > {
> > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip
*chip)
> > mtd->_read_oob = nand_read_oob;
> > mtd->_write_oob = nand_write_oob;
> > mtd->_sync = nand_sync;
> > - mtd->_lock = NULL;
> > - mtd->_unlock = NULL;
> > + mtd->_lock = nand_lock;
> > + mtd->_unlock = nand_unlock;
> > mtd->_suspend = nand_suspend;
> > mtd->_resume = nand_resume;
> > mtd->_reboot = nand_shutdown;
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 4ab9bcc..2430ecd 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1136,6 +1136,9 @@ struct nand_chip {
> > const struct nand_manufacturer *desc;
> > void *priv;
> > } manufacturer;
> > +
> > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>
> Please drop this _ prefix.
Drop _ prefix of _lock will get compile error due to there is already
defined "struct mutex lock" in struct nand_chip.
What about keep this _ prefix or patch it to blocklock/blockunlock,
i.e.,
int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi Miquel,
> > +/*
> > + * Macronix NAND supports Block Protection by Protectoin(PT) pin;
> > + * active high at power-on which protects the entire chip even the
#WP is
> > + * disabled. Lock/unlock protection area can be partition according
to
> > + * protection bits, i.e. upper 1/2 locked, upper 1/4 locked and so
on.
> > + */
> > +static int mxic_nand_lock(struct nand_chip *chip, loff_t ofs,
uint64_t len)
> > +{
> > + u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
> > + int ret;
> > +
> > + feature[0] = MXIC_BLOCK_PROTECTION_ALL_LOCK;
> > + nand_select_target(chip, 0);
> > + ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
> > + feature);
> > + nand_deselect_target(chip);
> > + if (ret)
> > + pr_err("%s all blocks failed\n", __func__);
> > +
> > + return ret;
> > +}
> > +
> > +static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs,
uint64_t len)
> > +{
> > + u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
> > + int ret;
> > +
> > + feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
> > + nand_select_target(chip, 0);
> > + ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
> > + feature);
> > + nand_deselect_target(chip);
> > + if (ret)
> > + pr_err("%s all blocks failed\n", __func__);
> > +
> > + return ret;
> > }
> >
> > +/*
> > + * Macronix NAND AC series support Block Protection by SET_FEATURES
> > + * to lock/unlock blocks.
> > + */
> > static int macronix_nand_init(struct nand_chip *chip)
> > {
> > + bool blockprotected = false;
> > +
> > if (nand_is_slc(chip))
> > chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
> >
> > - macronix_nand_fix_broken_get_timings(chip);
> > + if (macronix_nand_fix_broken_get_timings(chip))
> > + blockprotected = true;
>
> I don't like this at all :)
>
> Please create a helper which detects which part is broken/protected
> then create helpers to act in this case.
okay, will patch it to read default protected value (after power-on)
for protection function detection.
>
> If the list is absolutely identical, you can share the detection
> helper. Otherwise, if you think the list can diverge, please only share
> the list for now and create two detection helpers.
>
> > +
> > macronix_nand_onfi_init(chip);
> >
> > + if (blockprotected) {
> > + bitmap_set(chip->parameters.set_feature_list,
> > + ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
> > + bitmap_set(chip->parameters.get_feature_list,
> > + ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
> > +
> > + chip->_lock = mxic_nand_lock;
> > + chip->_unlock = mxic_nand_unlock;
> > + }
> > +
> > return 0;
> > }
> >
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi Mason,
[email protected] wrote on Mon, 17 Feb 2020 16:14:23 +0800:
> Hi Boris,
>
> >
> > > /* Set default functions */
> > > static void nand_set_defaults(struct nand_chip *chip)
> > > {
> > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip
> *chip)
> > > mtd->_read_oob = nand_read_oob;
> > > mtd->_write_oob = nand_write_oob;
> > > mtd->_sync = nand_sync;
> > > - mtd->_lock = NULL;
> > > - mtd->_unlock = NULL;
> > > + mtd->_lock = nand_lock;
> > > + mtd->_unlock = nand_unlock;
> > > mtd->_suspend = nand_suspend;
> > > mtd->_resume = nand_resume;
> > > mtd->_reboot = nand_shutdown;
> > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > > index 4ab9bcc..2430ecd 100644
> > > --- a/include/linux/mtd/rawnand.h
> > > +++ b/include/linux/mtd/rawnand.h
> > > @@ -1136,6 +1136,9 @@ struct nand_chip {
> > > const struct nand_manufacturer *desc;
> > > void *priv;
> > > } manufacturer;
> > > +
> > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> >
> > Please drop this _ prefix.
>
> Drop _ prefix of _lock will get compile error due to there is already
> defined "struct mutex lock" in struct nand_chip.
Right!
>
> What about keep this _ prefix or patch it to blocklock/blockunlock,
> i.e.,
> int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
What about lock_area() unlock_area() ? Seems more accurate to me, tell
me if I'm wrong.
>
>
> thanks for your time & comments.
> Mason
Thanks,
Miquèl
On Mon, 17 Feb 2020 10:01:24 +0100
Miquel Raynal <[email protected]> wrote:
> Hi Mason,
>
> [email protected] wrote on Mon, 17 Feb 2020 16:14:23 +0800:
>
> > Hi Boris,
> >
> > >
> > > > /* Set default functions */
> > > > static void nand_set_defaults(struct nand_chip *chip)
> > > > {
> > > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip
> > *chip)
> > > > mtd->_read_oob = nand_read_oob;
> > > > mtd->_write_oob = nand_write_oob;
> > > > mtd->_sync = nand_sync;
> > > > - mtd->_lock = NULL;
> > > > - mtd->_unlock = NULL;
> > > > + mtd->_lock = nand_lock;
> > > > + mtd->_unlock = nand_unlock;
> > > > mtd->_suspend = nand_suspend;
> > > > mtd->_resume = nand_resume;
> > > > mtd->_reboot = nand_shutdown;
> > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > > > index 4ab9bcc..2430ecd 100644
> > > > --- a/include/linux/mtd/rawnand.h
> > > > +++ b/include/linux/mtd/rawnand.h
> > > > @@ -1136,6 +1136,9 @@ struct nand_chip {
> > > > const struct nand_manufacturer *desc;
> > > > void *priv;
> > > > } manufacturer;
> > > > +
> > > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > >
> > > Please drop this _ prefix.
> >
> > Drop _ prefix of _lock will get compile error due to there is already
> > defined "struct mutex lock" in struct nand_chip.
>
> Right!
Or maybe move all hooks to a sub-struct (struct nand_chip_ops ops). I
had planned to do that in my nand_chip_legacy refactor but never did, so
maybe now is a good time.
>
> >
> > What about keep this _ prefix or patch it to blocklock/blockunlock,
> > i.e.,
> > int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>
> What about lock_area() unlock_area() ? Seems more accurate to me, tell
> me if I'm wrong.
Yep, definitely better.
Hi Miquel,
> > > > /* Set default functions */
> > > > static void nand_set_defaults(struct nand_chip *chip)
> > > > {
> > > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip
> > *chip)
> > > > mtd->_read_oob = nand_read_oob;
> > > > mtd->_write_oob = nand_write_oob;
> > > > mtd->_sync = nand_sync;
> > > > - mtd->_lock = NULL;
> > > > - mtd->_unlock = NULL;
> > > > + mtd->_lock = nand_lock;
> > > > + mtd->_unlock = nand_unlock;
> > > > mtd->_suspend = nand_suspend;
> > > > mtd->_resume = nand_resume;
> > > > mtd->_reboot = nand_shutdown;
> > > > diff --git a/include/linux/mtd/rawnand.h
b/include/linux/mtd/rawnand.h
> > > > index 4ab9bcc..2430ecd 100644
> > > > --- a/include/linux/mtd/rawnand.h
> > > > +++ b/include/linux/mtd/rawnand.h
> > > > @@ -1136,6 +1136,9 @@ struct nand_chip {
> > > > const struct nand_manufacturer *desc;
> > > > void *priv;
> > > > } manufacturer;
> > > > +
> > > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t
len);
> > > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t
len);
> > >
> > > Please drop this _ prefix.
> >
> > Drop _ prefix of _lock will get compile error due to there is already
> > defined "struct mutex lock" in struct nand_chip.
>
> Right!
>
> >
> > What about keep this _ prefix or patch it to blocklock/blockunlock,
> > i.e.,
> > int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> > int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>
> What about lock_area() unlock_area() ? Seems more accurate to me, tell
> me if I'm wrong.
yup, you are right!
Using lock/unlock_area is better, will patch it.
thanks for your comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi Miquel,
> > Macronix AD series support deep power down mode for a minimum
> > power consumption state.
> >
> > Patch nand_suspend() & nand_resume() by Macronix specific
> > deep power down mode command and exit it.
> >
> > Signed-off-by: Mason Yang <[email protected]>
> > ---
> > drivers/mtd/nand/raw/nand_macronix.c | 72
+++++++++++++++++++++++++++++++++++-
> > 1 file changed, 70 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_macronix.c
b/drivers/mtd/nand/raw/
> nand_macronix.c
> > index 13929bf..3098bc0 100644
> > --- a/drivers/mtd/nand/raw/nand_macronix.c
> > +++ b/drivers/mtd/nand/raw/nand_macronix.c
> > @@ -15,6 +15,8 @@
> > #define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
> > #define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
> >
> > +#define NAND_CMD_POWER_DOWN 0xB9
>
> I suppose this value is Macronix specific, and hence should have a
> MACRONIX_ or MXIC_ prefix instead of NAND_.
okay, will patch it to
#define MXIC_CMD_POWER_DOWN 0xB9
>
> > +
> > struct nand_onfi_vendor_macronix {
> > u8 reserved;
> > u8 reliability_func;
> > @@ -137,13 +139,66 @@ static int mxic_nand_unlock(struct nand_chip
*chip,
> loff_t ofs, uint64_t len)
> > return ret;
> > }
> >
> > +int nand_power_down_op(struct nand_chip *chip)
> > +{
> > + int ret;
> > +
> > + if (nand_has_exec_op(chip)) {
> > + struct nand_op_instr instrs[] = {
> > + NAND_OP_CMD(NAND_CMD_POWER_DOWN, 0),
> > + };
> > +
> > + struct nand_operation op = NAND_OPERATION(chip->cur_cs,
instrs);
> > +
> > + ret = nand_exec_op(chip, &op);
> > + if (ret)
> > + return ret;
> > +
> > + } else {
> > + chip->legacy.cmdfunc(chip, NAND_CMD_POWER_DOWN, -1, -1);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mxic_nand_suspend(struct nand_chip *chip)
> > +{
> > + int ret;
> > +
> > + nand_select_target(chip, 0);
> > + ret = nand_power_down_op(chip);
> > + if (ret < 0)
> > + pr_err("%s called for chip into suspend failed\n", __func__);
>
> What about something more specific?
>
> "Suspending MXIC NAND chip failed (%)\n", ret
okay, patch it with this sentence, thanks.
>
> > + nand_deselect_target(chip);
> > +
> > + return ret;
> > +}
> > +
> > +static void mxic_nand_resume(struct nand_chip *chip)
> > +{
> > + /*
> > + * Toggle #CS pin to resume NAND device and don't care
> > + * of the others CLE, #WE, #RE pins status.
> > + * Here sending power down command to toggle #CS line.
>
> The first sentence seems right, the second could be upgraded:
>
> The purpose of doing a power down operation is just to
> ensure some bytes will be sent over the NAND bus so that #CS
> gets toggled because this is why the chip is woken up.
> The content of the bytes sent on the NAND bus are not
> relevant at this time. Sending bytes on the bus is mandatory
> for a lot of NAND controllers otherwise they are not able to
> just assert/de-assert #CS.
okay, will patch the second sentence based on your comments.
i.e,.
/*
* Toggle #CS pin to resume NAND device and don't care
* of the others CLE, #WE, #RE pins status.
* A NAND controller ensure it is able to assert/de-assert #CS
* by sending any byte over the NAND bus.
* i.e.,
* NAND power down command or reset command.
*/
>
> > + */
> > + nand_select_target(chip, 0);
> > + nand_power_down_op(chip);
>
> Are you sure sending a power_down_op will not be interpreted by the
> chip?
yes, sure !
>
> I would expect a sleeping delay here, even small.
okay, will patch it.
>
> > + nand_deselect_target(chip);
> > +}
> > +
> > /*
> > - * Macronix NAND AC series support Block Protection by SET_FEATURES
> > + * Macronix NAND AC & AD series support Block Protection by
SET_FEATURES
> > * to lock/unlock blocks.
> > */
> > static int macronix_nand_init(struct nand_chip *chip)
> > {
> > - bool blockprotected = false;
> > + unsigned int i;
> > + bool blockprotected = false, powerdown = false;
> > + static const char * const power_down_dev[] = {
> > + "MX30LF1G28AD",
> > + "MX30LF2G28AD",
> > + "MX30LF4G28AD",
> > + };
> >
> > if (nand_is_slc(chip))
> > chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
> > @@ -153,6 +208,14 @@ static int macronix_nand_init(struct nand_chip
*chip)
> >
> > macronix_nand_onfi_init(chip);
> >
> > + for (i = 0; i < ARRAY_SIZE(power_down_dev); i++) {
> > + if (!strcmp(power_down_dev[i], chip->parameters.model)) {
> > + blockprotected = true;
> > + powerdown = true;
> > + break;
> > + }
> > + }
> > +
> > if (blockprotected) {
> > bitmap_set(chip->parameters.set_feature_list,
> > ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
> > @@ -163,6 +226,11 @@ static int macronix_nand_init(struct nand_chip
*chip)
> > chip->_unlock = mxic_nand_unlock;
> > }
> >
> > + if (powerdown) {
> > + chip->_suspend = mxic_nand_suspend;
> > + chip->_resume = mxic_nand_resume;
> > + }
>
> See my comment on patch 2.
ok, got it.
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================