add the GPMI controller driver for IMX23/IMX28

Hi,

I'm still not happy with the rom-helper code that IMO does not belong in this driver.
You could enable CONFIG_MTD_CMDLINE_PARTS and add something like:
"mtdparts=gpmi-nfc-main:2m(gpmi-nfc-0-boot)ro,-(gpmi-nfc-general-use)" to the kernel cmdline to achieve the same without any special code inside the chip driver.

Also, I would integrate the code from the hal-*.c files into the main file and remove all the function hooks, since the functions are the same for all the current variants anyway. You might hook the 'begin()' and 'is_ready()' functions which are the only ones that are different in the current variants so that the distinction can be made once upon initialization rather than on every function call:
+static int mx23_is_ready(struct gpmi_nfc_data *this, unsigned chip) +{

+	struct resources  *resources = &this->resources;
+	uint32_t          mask;
+	uint32_t          reg;

+
+ mask = MX23_BM_GPMI_DEBUG_READY0 << chip;
+ reg = __raw_readl(resources->gpmi_regs + HW_GPMI_DEBUG);
+
+ return !!(reg & mask);
+}
+
+static void mx28_begin(struct gpmi_nfc_data *this)
+{
+ struct resources *resources = &this->resources;
{...]
+}
+
+static int mx28_is_ready(struct gpmi_nfc_data *this, unsigned chip) +{

+	struct resources  *resources = &this->resources;
+	uint32_t          mask;
+	uint32_t          reg;

+
+ mask = MX28_BF_GPMI_STAT_READY_BUSY(1 << chip);
+ reg = __raw_readl(resources->gpmi_regs + HW_GPMI_STAT);
+
+ return !!(reg & mask);
+}
[...]
+static int gpmi_nfc_probe(struct platform_device *pdev) [...]
+ if (CPU_IS_MX23()) {
+ this->is_ready = mx23_is_ready;
+ } else if (CPU_IS_MX28()) {
+ this->is_ready = mx28_is_ready;
+ this->begin = mx28_begin;
+ }
Since the begin() function is a NOP for i.MX23, the function pointer could be left unassigned and the function only be called if the pointer is not NULL or an empty function could be assigned.

Further I wouldn't name the macro for distinguishing the different controller variants CPU_IS_... but something like GPMI_IS

Lothar Wa?mann

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

Hi Lothar:

Could you tell me which part of the rom-helper code is not belong the driver?

The original code does have the command-line-partition parsing code,

I removed it. I think I did a wrong thing.

I found the code is needed yesterday. The android system needs a different partition layout. I had to revert the code back.

thanks for your advice.

This driver will serve for many platforms, not only the imx23 and imx28. I am merging the imx508 code to the driver.
I feel lucky that i did not merge all the code into the main file, which will
make the code mess.

But still thanks for your advice.

ok. This is a small change. I will do it in the next version. thanks

Huang Shijie

Hi,

All of it. I don't see a point in having it, when all that it does can be achieved with standard functions already.

You would probably end up with some more functions that are implemented for the different variants. They could be prefixed with the SoC name and selected in the probe() function depending on the platform id. I don't see why this should get messy.

If the imx508 driver is so much different from the other variants, that merging it gets too messy, it's probably better to not merge it with the other variants anyway.

Lothar Wa?mann

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

Hi Lothar:

I am afraid most of the rom-help code can not be removed, the reasons are:
[1] imx23 does not need the swap-block-mark feature. The swap-block-mark feature is used
by the BCH hardware ecc-correction module. But the imx28 does need
the swap-block-mark feature.
Why? the firmwares in both chips are a little different. The
rom-help code is used to distinguish
this, and _DO_ some necessary initializations. After the
initialization, the NAND_SCAN will works.

[2] In the NAND boot mode, we also need to check the swap-block-mark to do the right ECC read pages.

BUT the default partition layout code really can be removed. It somehow can be regards as not belonged to
the driver.

thanks.

I really do not like to package all the code into one file which makes the file bigger and bigger.
Frankly speaking, I regard it as a bad idea. I think it's messy.

imx508 will support DDR mode for ONFI NAND and TOGGLE nand which need to do lot of TIMING initialization for both mode.
It is a long initialization code. Keep the code in separate file is to make the code tidy. Do you think it's a
grace method to keep it in one file? I do not think so. :)

yes, I can use the PREFIXED name for different variants. But I think that will create more code.
The current *hal.c only contains the different implementation for variants.

It just needs a separate hal-508.c file.

Thanks a lot.

Best Regards
Huang Shijie