[PATCH v3 3/7] eSPI: add eSPI controller support

[Hu Mingkai-B21284]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru at gmail.com]
> Sent: Friday, October 01, 2010 7:22 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-dev at ozlabs.org; spi-devel-general at lists.sourceforge.net; linux-
> mtd at lists.infradead.org; Gala Kumar-B11780
> Subject: Re: [PATCH v3 3/7] eSPI: add eSPI controller support
> 
> Hello Mingkai,
> 
> There are mostly cosmetic comments down below.
> 
> > +		/* spin until TX is done */
> > +		while (((events = mpc8xxx_spi_read_reg(&reg_base->event))
> > +					& SPIE_NF) == 0)
> > +			cpu_relax();
> 
> This is dangerous. There's a handy spin_event_timeout() in asm/delay.h.
> 

When timeout, can I use return in the interrupt function directly like this?

if (!(events & SPIE_NF)) {
        int ret;
        /* spin until TX is done */
        ret = spin_event_timeout(((events = mpc8xxx_spi_read_reg(
                        &reg_base->event)) & SPIE_NF) == 0, 1000, 0);
        if (!ret) {
                dev_err(mspi->dev, "tired waiting for SPIE_NF\n");
                return;
        }
}

> > +}
> > +
> > +static const struct of_device_id of_fsl_espi_match[] = {
> > +	{ .compatible = "fsl,mpc8536-espi" },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(of, of_fsl_espi_match);
> > +
> > +static struct of_platform_driver fsl_espi_driver = {
> > +	.driver = {
> > +		.name = "fsl_espi",
> > +		.owner = THIS_MODULE,
> > +		.of_match_table = of_fsl_espi_match,
> > +	},
> > +	.probe		= of_fsl_espi_probe,
> > +	.remove		= __devexit_p(of_fsl_espi_remove),
> > +};
> > +
> > +static int __init fsl_espi_init(void) {
> > +	return of_register_platform_driver(&fsl_espi_driver);
> > +}
> > +module_init(fsl_espi_init);
> > +
> > +static void __exit fsl_espi_exit(void) {
> > +	of_unregister_platform_driver(&fsl_espi_driver);
> > +}
> > +module_exit(fsl_espi_exit);
> > +
> > +MODULE_AUTHOR("Mingkai Hu");
> > +MODULE_DESCRIPTION("Enhanced Freescale SPI Driver");
> 
> This sounds like that this is an enhanced version of the Freescale SPI driver,
> which it is not. ;-)
> 

I quoted from the UM, maybe the enhancement is the controller takes over the
CS signal from the HW point view.

I changed all the other code according to your comments.

Thanks,
Mingkai