Hello, Some purely cosmetic suggestions below.
On Fri, 10 May 2019 15:41:02 +0800 Mason Yang <[email protected]> wrote: > + if (ret) > + pr_err("set feature failed to read retry moded:%d\n", mode); I don't know what is the policy in the MTD/NAND subsystem, but shouldn't you be using dev_err() instead of pr_err() here to have a nice prefix for the message ? dev_err(&nand_to_mtd(chip)->dev, "set feature ..", mode); > +static void macronix_nand_onfi_init(struct nand_chip *chip) > +{ > + struct nand_parameters *p = &chip->parameters; > + > + if (p->onfi) { Change to: if (!p->onfi) return; This way the rest of the function can save one level of indentation. > + struct nand_onfi_vendor_macronix *mxic = > + (void *)p->onfi->vendor; > + > + if (mxic->reliability_func & MACRONIX_READ_RETRY_BIT) { Change to: if (mxic->reliability_func & MACRONIX_READ_RETRY_BIT == 0) return; And the rest of the function can save one level of indentation. > + chip->read_retries = MACRONIX_READ_RETRY_MODE + 1; > + chip->setup_read_retry = > + macronix_nand_setup_read_retry; > + if (p->supports_set_get_features) { > + set_bit(ONFI_FEATURE_ADDR_READ_RETRY, > + p->set_feature_list); > + set_bit(ONFI_FEATURE_ADDR_READ_RETRY, > + p->get_feature_list); > + } Which will require less wrapping in those lines that are already at the third indentation level. To me, it is also more logical: we exclude the cases we are not interested in and return early, and then if we are still in the case we are interested, we handle it. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
