On 03/03/2017 08:38 PM, Sergio Prado wrote:

On Fri, Mar 03, 2017 at 11:14:29AM +0900, Jaehoon Chung wrote:

quoted

On 03/02/2017 10:18 AM, Sergio Prado wrote:

quoted

Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
tree.

Signed-off-by: Sergio Prado <redacted>
---
 drivers/mmc/host/s3cmci.c | 298 ++++++++++++++++++++++++----------------------
 drivers/mmc/host/s3cmci.h |   3 +-
 2 files changed, 158 insertions(+), 143 deletions(-)

diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index 7a173f8c455b..d066dbdb957c 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c

@@ -24,6 +24,10 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include <plat/gpio-cfg.h>
 #include <mach/dma.h>

@@ -128,6 +132,22 @@ enum dbg_channels {
 	dbg_conf  = (1 << 8),
 };
 
+struct s3cmci_variant_data {
+	int s3c2440_compatible;
+};

I didn't understand why this structure needs.

Before this patch,
host->is2440;

After this patch,
host->variant->s3c2440_compatible;

Just add the one pointer for checking s3c2400 compatible..
Is it really meaningful?
(I didn't read the previous comments fully.)

Although just a pointer would be enought, having a structure makes it
more flexible to extend it in the future.

If you will add the other members in this structure, it's ok.
But if it's only for compatible, i don't agree this.

Best Regards,
Jaehoon Chung

Best regards,

Sergio Prado