Hi Miquel,
> > +
> > +static void mxic_nand_select_chip(struct nand_chip *chip, int chipnr)
>
> _select_target() is preferred now
Do you mean I implement mxic_nand_select_target() to control #CS ?
If so, I need to call mxic_nand_select_target( ) to control #CS ON
and then #CS OFF in _exec_op() due to nand_select_target()<in nand_base,c>
is still calling chip->legacy.select_chip ?
>
> > +{
> > + struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> > +
> > + switch (chipnr) {
> > + case 0:
> > + case 1:
> > + writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> > + writel(HC_CFG_MAN_CS_ASSERT | readl(mxic->mfd->regs + HC_CFG),
> > + mxic->mfd->regs + HC_CFG);
>
> In both case I would prefer a:
>
> reg = readl(...);
> reg &= ~xxx;
> reg |= yyy;
> writel(reg, ...);
>
> Much easier to read.
>
> > + break;
> > +
> > + case -1:
> > + writel(~HC_CFG_MAN_CS_ASSERT & readl(mxic->mfd->regs + HC_CFG),
> > + mxic->mfd->regs + HC_CFG);
> > + writel(0, mxic->mfd->regs + HC_EN);
> > + break;
> > +
> > + default:
>
> Error?
>
> > + break;
> > + }
> > +}
> > +
> > +static int mx25f0a_nand_probe(struct platform_device *pdev)
> > +{
> > + struct mtd_info *mtd;
> > + struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
> > + struct mxic_nand_ctlr *mxic;
> > + struct nand_chip *nand_chip;
> > + int err;
> > +
> > + mxic = devm_kzalloc(&pdev->dev, sizeof(struct mxic_nand_ctlr),
> > + GFP_KERNEL);
>
> mxic for a NAND controller structure is probably not a name meaningful
> enough.
>
> > + if (!mxic)
> > + return -ENOMEM;
> > +
> > + nand_chip = &mxic->nand;
> > + mtd = nand_to_mtd(nand_chip);
> > + mtd->dev.parent = pdev->dev.parent;
> > + nand_chip->ecc.priv = NULL;
> > + nand_set_flash_node(nand_chip, pdev->dev.parent->of_node);
> > + nand_chip->priv = mxic;
> > +
> > + mxic->mfd = mfd;
> > +
> > + nand_chip->legacy.select_chip = mxic_nand_select_chip;
>
> Please don't implement legacy interfaces. You can check in
> marvell_nand.c how this is handled now:
>
> b25251414f6e mtd: rawnand: marvell: Stop implementing ->select_chip()
>
Does it mean chip->legacy.select_chip() will phase-out ?
thanks & best regards,
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.
=====================================================================
