Re: [PATCH 2/4] mtd: spi-nor: add a new data structrue spi_nor{}

From: Marek Vasut <marex@denx.de>
To: Huang Shijie <b32955@freescale.com>
Cc: "broonie@linaro.org" <broonie@linaro.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"Gupta, Pekon" <pekon@ti.com>,
	"Poddar, Sourav" <sourav.poddar@ti.com>,
	"computersforpeace@gmail.com" <computersforpeace@gmail.com>,
	"dwmw2@infradead.org" <dwmw2@infradead.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/4] mtd: spi-nor: add a new data structrue spi_nor{}
Date: Wed, 27 Nov 2013 10:32:13 +0100	[thread overview]
Message-ID: <201311271032.13259.marex@denx.de> (raw)
In-Reply-To: <52957690.60102@freescale.com>

Dear Huang Shijie,

> 于 2013年11月26日 19:42, Gupta, Pekon 写道:
> >> From: Huang Shijie [mailto:b32955@freescale.com]
> > 
> > [...]
> > 
> >> +#define	MAX_CMD_SIZE		6
> >> +
> >> +enum read_type {
> >> +	M25P80_NORMAL = 0,
> >> +	M25P80_FAST,
> >> +	M25P80_QUAD,
> >> +};
> > 
> > Sorry. no 'M25P80' suffix here this is spi-nor.h :-)
> 
> ok. thanks. :)
> 
> >> +
> >> +struct spi_nor {
> >> +	struct mutex		lock;
> >> +	struct mtd_info		mtd;
> > 
> > mtd_info should not be present here. Rather it should be other way round
> > 'mtd_info->priv = (struct spi_nor *) spi_nor;
> 
> put the mtd here can make code simple,

The MTD API functions will pass you the struct mtd_info anyway, so you will need 
to implement mtdinfo_to_yourdriverdata() function, no need for duplication.

> do David/Brian have any comment about this?
> If all object to put the mtd here, i will change it.
> 
> >> +	struct device		*dev;
> > 
> > Again, spi_nor would be a MTD device, not a new type of device on its
> > own. Thus you should use, mtd_info->dev.
> 
> this dev pointer is not from the mtd_info->dev, it from the spi_device
> or other spi nor device .

So this is your own device pointer or ... what kind of device pointer?
[...]

Best regards,
Marek Vasut