[PATCH 1/3] spi_topcliff_pch: support new device ML7213

[PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Support ML7213 device of OKI SEMICONDUCTOR.
ML7213 is companion chip of Intel Atom E6xx series for IVI(In-Vehicle Infotainment).
ML7213 is completely compatible for Intel EG20T PCH.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/spi/spi_topcliff_pch.c |  293 +++++++++++++++++++++++-----------------
 1 files changed, 172 insertions(+), 121 deletions(-)

diff --git a/drivers/spi/spi_topcliff_pch.c b/drivers/spi/spi_topcliff_pch.c
index 58e187f..18e077b 100644
--- a/drivers/spi/spi_topcliff_pch.c
+++ b/drivers/spi/spi_topcliff_pch.c
@@ -35,6 +35,7 @@
 #define PCH_SPDRR		0x10	/* SPI read data register */
 #define PCH_SSNXCR		0x18	/* SSN Expand Control Register */
 #define PCH_SRST		0x1C	/* SPI reset register */
+#define PCH_SPI_ADDRESS_SIZE	0x20
 
 #define PCH_SPSR_TFD		0x000007C0
 #define PCH_SPSR_RFD		0x0000F800
@@ -88,6 +89,16 @@
 #define PCH_CLOCK_HZ		50000000
 #define PCH_MAX_SPBR		1023
 
+/* Definition for ML7213 by OKI SEMICONDUCTOR */
+#define PCI_VENDOR_ID_ROHM		0x10DB
+#define PCI_DEVICE_ID_ML7213_SPI	0x802c
+
+/*
+Set the number of SPI instance max
+Intel EG20T PCH :		1ch
+OKI SEMICONDUCTOR ML7213 IOH :	2ch
+*/
+#define PCH_SPI_MAX_DEV			2
 
 /**
  * struct pch_spi_data - Holds the SPI channel specific details
@@ -153,18 +164,21 @@ struct pch_spi_data {
  * @pci_req_sts:	Status of pci_request_regions
  * @suspend_sts:	Status of suspend
  * @data:		Pointer to SPI channel data structure
+ * @num:		The number of SPI device instance
  */
 struct pch_spi_board_data {
 	struct pci_dev *pdev;
 	u8 irq_reg_sts;
 	u8 pci_req_sts;
 	u8 suspend_sts;
-	struct pch_spi_data *data;
+	struct pch_spi_data *data[PCH_SPI_MAX_DEV];
+	int num;
 };
 
 static struct pci_device_id pch_spi_pcidev_id[] = {
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_GE_SPI)},
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_GE_SPI),    1, },
+	{ PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_SPI), 2, },
+	{ }
 };
 
 /**
@@ -267,7 +281,7 @@ static void pch_spi_handler_sub(struct pch_spi_data *data, u32 reg_spsr_val,
 	if (reg_spsr_val & SPSR_FI_BIT) {
 		/* disable FI & RFI interrupts */
 		pch_spi_setclr_reg(data->master, PCH_SPCR, 0,
-				   SPCR_FIE_BIT | SPCR_TFIE_BIT);
+				   SPCR_FIE_BIT | SPCR_RFIE_BIT);
 
 		/* transfer is completed;inform pch_spi_process_messages */
 		data->transfer_complete = true;
@@ -288,6 +302,7 @@ static irqreturn_t pch_spi_handler(int irq, void *dev_id)
 	void __iomem *io_remap_addr;
 	irqreturn_t ret = IRQ_NONE;
 	struct pch_spi_board_data *board_dat = dev_id;
+	int i;
 
 	if (board_dat->suspend_sts) {
 		dev_dbg(&board_dat->pdev->dev,
@@ -295,21 +310,22 @@ static irqreturn_t pch_spi_handler(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	data = board_dat->data;
-	io_remap_addr = data->io_remap_addr;
-	spsr = io_remap_addr + PCH_SPSR;
+	for (i = 0; i < board_dat->num; i++) {
+		data = board_dat->data[i];
+		io_remap_addr = data->io_remap_addr;
+		spsr = io_remap_addr + PCH_SPSR;
 
-	reg_spsr_val = ioread32(spsr);
+		reg_spsr_val = ioread32(spsr);
 
-	/* Check if the interrupt is for SPI device */
-	if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
-		pch_spi_handler_sub(data, reg_spsr_val, io_remap_addr);
-		ret = IRQ_HANDLED;
-	}
-
-	dev_dbg(&board_dat->pdev->dev, "%s EXIT return value=%d\n",
-		__func__, ret);
+		/* Check if the interrupt is for SPI device */
+		if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
+			pch_spi_handler_sub(data, reg_spsr_val, io_remap_addr);
+			ret = IRQ_HANDLED;
+		}
 
+		dev_dbg(&board_dat->pdev->dev, "%s EXIT return value=%d\n",
+			__func__, ret);
+	}
 	return ret;
 }
 
@@ -679,11 +695,11 @@ static void pch_spi_set_ir(struct pch_spi_data *data)
 	if ((data->bpw_len) > PCH_MAX_FIFO_DEPTH) {
 		/* set receive threhold to PCH_RX_THOLD */
 		pch_spi_setclr_reg(data->master, PCH_SPCR,
-				   PCH_RX_THOLD << SPCR_TFIC_FIELD,
-				   ~MASK_TFIC_SPCR_BITS);
+				   PCH_RX_THOLD << SPCR_RFIC_FIELD,
+				   ~MASK_RFIC_SPCR_BITS);
 		/* enable FI and RFI interrupts */
 		pch_spi_setclr_reg(data->master, PCH_SPCR,
-				   SPCR_RFIE_BIT | SPCR_TFIE_BIT, 0);
+				   SPCR_RFIE_BIT | SPCR_FIE_BIT, 0);
 	} else {
 		/* set receive threhold to maximum */
 		pch_spi_setclr_reg(data->master, PCH_SPCR,
@@ -870,37 +886,46 @@ static void pch_spi_process_messages(struct work_struct *pwork)
 
 static void pch_spi_free_resources(struct pch_spi_board_data *board_dat)
 {
+	int i;
+
 	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);
 
 	/* free workqueue */
-	if (board_dat->data->wk != NULL) {
-		destroy_workqueue(board_dat->data->wk);
-		board_dat->data->wk = NULL;
-		dev_dbg(&board_dat->pdev->dev,
-			"%s destroy_workqueue invoked successfully\n",
-			__func__);
+	for (i = 0; i < board_dat->num; i++) {
+		if (board_dat->data[i]->wk != NULL) {
+			destroy_workqueue(board_dat->data[i]->wk);
+			board_dat->data[i]->wk = NULL;
+			dev_dbg(&board_dat->pdev->dev,
+				"%s destroy_workqueue invoked successfully\n",
+				__func__);
+		}
 	}
 
 	/* disable interrupts & free IRQ */
 	if (board_dat->irq_reg_sts) {
-		/* disable interrupts */
-		pch_spi_setclr_reg(board_dat->data->master, PCH_SPCR, 0,
-				   PCH_ALL);
+		for (i = 0; i < board_dat->num; i++) {
+			/* disable interrupts */
+			pch_spi_setclr_reg(board_dat->data[i]->master, PCH_SPCR, 0,
+					   PCH_ALL);
 
-		/* free IRQ */
-		free_irq(board_dat->pdev->irq, board_dat);
+			if (i == (board_dat->num - 1)) {
+				/* free IRQ */
+				free_irq(board_dat->pdev->irq, board_dat);
 
-		dev_dbg(&board_dat->pdev->dev,
-			"%s free_irq invoked successfully\n", __func__);
+				dev_dbg(&board_dat->pdev->dev,
+					"%s free_irq invoked successfully\n", __func__);
 
-		board_dat->irq_reg_sts = false;
+				board_dat->irq_reg_sts = false;
+			}
+		}
 	}
 
 	/* unmap PCI base address */
-	if (board_dat->data->io_remap_addr != 0) {
-		pci_iounmap(board_dat->pdev, board_dat->data->io_remap_addr);
+	if (board_dat->data[0]->io_remap_addr != 0) {
+		pci_iounmap(board_dat->pdev, board_dat->data[0]->io_remap_addr);
 
-		board_dat->data->io_remap_addr = 0;
+		for (i = 0; i < board_dat->num; i++)
+			board_dat->data[i]->io_remap_addr = 0;
 
 		dev_dbg(&board_dat->pdev->dev,
 			"%s pci_iounmap invoked successfully\n", __func__);
@@ -920,19 +945,23 @@ static int pch_spi_get_resources(struct pch_spi_board_data *board_dat)
 {
 	void __iomem *io_remap_addr;
 	int retval;
+	int i;
+
 	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);
 
-	/* create workqueue */
-	board_dat->data->wk = create_singlethread_workqueue(KBUILD_MODNAME);
-	if (!board_dat->data->wk) {
-		dev_err(&board_dat->pdev->dev,
-			"%s create_singlet hread_workqueue failed\n", __func__);
-		retval = -EBUSY;
-		goto err_return;
-	}
+	for (i = 0; i < board_dat->num; i++) {
+		/* create workqueue */
+		board_dat->data[i]->wk = create_singlethread_workqueue(KBUILD_MODNAME);
+		if (!board_dat->data[i]->wk) {
+			dev_err(&board_dat->pdev->dev,
+				"%s create_singlet hread_workqueue failed\n", __func__);
+			retval = -EBUSY;
+			goto err_return;
+		}
 
-	dev_dbg(&board_dat->pdev->dev,
-		"%s create_singlethread_workqueue success\n", __func__);
+		dev_dbg(&board_dat->pdev->dev,
+			"%s create_singlethread_workqueue success\n", __func__);
+	}
 
 	retval = pci_request_regions(board_dat->pdev, KBUILD_MODNAME);
 	if (retval != 0) {
@@ -951,13 +980,15 @@ static int pch_spi_get_resources(struct pch_spi_board_data *board_dat)
 		goto err_return;
 	}
 
-	/* calculate base address for all channels */
-	board_dat->data->io_remap_addr = io_remap_addr;
+	for (i = 0; i < board_dat->num; i++) {
+		/* calculate base address for all channels */
+		board_dat->data[i]->io_remap_addr = io_remap_addr + (PCH_SPI_ADDRESS_SIZE * i);
 
-	/* reset PCH SPI h/w */
-	pch_spi_reset(board_dat->data->master);
-	dev_dbg(&board_dat->pdev->dev,
-		"%s pch_spi_reset invoked successfully\n", __func__);
+		/* reset PCH SPI h/w */
+		pch_spi_reset(board_dat->data[i]->master);
+		dev_dbg(&board_dat->pdev->dev,
+			"%s pch_spi_reset invoked successfully\n", __func__);
+	}
 
 	/* register IRQ */
 	retval = request_irq(board_dat->pdev->irq, pch_spi_handler,
@@ -989,10 +1020,11 @@ err_return:
 static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 
-	struct spi_master *master;
+	struct spi_master *master[PCH_SPI_MAX_DEV];
 
 	struct pch_spi_board_data *board_dat;
 	int retval;
+	int i, j;
 
 	dev_dbg(&pdev->dev, "%s ENTRY\n", __func__);
 
@@ -1021,37 +1053,40 @@ static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		__func__, retval);
 
 	board_dat->pdev = pdev;
+	board_dat->num = id->driver_data;
+
+	for (i = 0; i < board_dat->num; i++) {
+		/* alllocate memory for SPI master */
+		master[i] = spi_alloc_master(&pdev->dev, sizeof(struct pch_spi_data));
+		if (master[i] == NULL) {
+			retval = -ENOMEM;
+			dev_err(&pdev->dev, "%s Fail.\n", __func__);
+			goto err_spi_alloc_master;
+		}
 
-	/* alllocate memory for SPI master */
-	master = spi_alloc_master(&pdev->dev, sizeof(struct pch_spi_data));
-	if (master == NULL) {
-		retval = -ENOMEM;
-		dev_err(&pdev->dev, "%s Fail.\n", __func__);
-		goto err_spi_alloc_master;
-	}
-
-	dev_dbg(&pdev->dev,
-		"%s spi_alloc_master returned non NULL\n", __func__);
+		dev_dbg(&pdev->dev,
+			"%s spi_alloc_master returned non NULL\n", __func__);
 
-	/* initialize members of SPI master */
-	master->bus_num = -1;
-	master->num_chipselect = PCH_MAX_CS;
-	master->setup = pch_spi_setup;
-	master->transfer = pch_spi_transfer;
-	dev_dbg(&pdev->dev,
-		"%s transfer member of SPI master initialized\n", __func__);
+		/* initialize members of SPI master */
+		master[i]->bus_num = -1;
+		master[i]->num_chipselect = PCH_MAX_CS;
+		master[i]->setup = pch_spi_setup;
+		master[i]->transfer = pch_spi_transfer;
+		dev_dbg(&pdev->dev,
+			"%s transfer member of SPI master initialized\n", __func__);
 
-	board_dat->data = spi_master_get_devdata(master);
+		board_dat->data[i] = spi_master_get_devdata(master[i]);
 
-	board_dat->data->master = master;
-	board_dat->data->n_curnt_chip = 255;
-	board_dat->data->board_dat = board_dat;
-	board_dat->data->status = STATUS_RUNNING;
+		board_dat->data[i]->master = master[i];
+		board_dat->data[i]->n_curnt_chip = 255;
+		board_dat->data[i]->board_dat = board_dat;
+		board_dat->data[i]->status = STATUS_RUNNING;
 
-	INIT_LIST_HEAD(&board_dat->data->queue);
-	spin_lock_init(&board_dat->data->lock);
-	INIT_WORK(&board_dat->data->work, pch_spi_process_messages);
-	init_waitqueue_head(&board_dat->data->wait);
+		INIT_LIST_HEAD(&board_dat->data[i]->queue);
+		spin_lock_init(&board_dat->data[i]->lock);
+		INIT_WORK(&board_dat->data[i]->work, pch_spi_process_messages);
+		init_waitqueue_head(&board_dat->data[i]->wait);
+	}
 
 	/* allocate resources for PCH SPI */
 	retval = pch_spi_get_resources(board_dat);
@@ -1068,29 +1103,32 @@ static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	dev_dbg(&pdev->dev, "%s invoked pci_set_drvdata\n", __func__);
 
 	/* set master mode */
-	pch_spi_set_master_mode(master);
-	dev_dbg(&pdev->dev,
-		"%s invoked pch_spi_set_master_mode\n", __func__);
+	for (i = 0; i < board_dat->num; i++) {
+		pch_spi_set_master_mode(master[i]);
+		dev_dbg(&pdev->dev,
+			"%s invoked pch_spi_set_master_mode\n", __func__);
+
+		/* Register the controller with the SPI core. */
+		retval = spi_register_master(master[i]);
+		if (retval != 0) {
+			dev_err(&pdev->dev,
+				"%s spi_register_master FAILED\n", __func__);
+			goto err_spi_reg_master;
+		}
 
-	/* Register the controller with the SPI core. */
-	retval = spi_register_master(master);
-	if (retval != 0) {
-		dev_err(&pdev->dev,
-			"%s spi_register_master FAILED\n", __func__);
-		goto err_spi_reg_master;
+		dev_dbg(&pdev->dev, "%s spi_register_master returned=%d\n",
+			__func__, retval);
 	}
 
-	dev_dbg(&pdev->dev, "%s spi_register_master returned=%d\n",
-		__func__, retval);
-
-
 	return 0;
 
 err_spi_reg_master:
-	spi_unregister_master(master);
+	for (j = 0; j < i; j++)
+		spi_unregister_master(master[j]);
 err_spi_get_resources:
 err_spi_alloc_master:
-	spi_master_put(master);
+	for (j = 0; j < i; j++)
+		spi_master_put(master[j]);
 	pci_disable_device(pdev);
 err_pci_en_device:
 	kfree(board_dat);
@@ -1102,6 +1140,7 @@ static void pch_spi_remove(struct pci_dev *pdev)
 {
 	struct pch_spi_board_data *board_dat = pci_get_drvdata(pdev);
 	int count;
+	int i;
 
 	dev_dbg(&pdev->dev, "%s ENTRY\n", __func__);
 
@@ -1113,22 +1152,26 @@ static void pch_spi_remove(struct pci_dev *pdev)
 
 	/* check for any pending messages; no action is taken if the queue
 	 * is still full; but at least we tried.  Unload anyway */
-	count = 500;
-	spin_lock(&board_dat->data->lock);
-	board_dat->data->status = STATUS_EXITING;
-	while ((list_empty(&board_dat->data->queue) == 0) && --count) {
-		dev_dbg(&board_dat->pdev->dev, "%s :queue not empty\n",
-			__func__);
-		spin_unlock(&board_dat->data->lock);
-		msleep(PCH_SLEEP_TIME);
-		spin_lock(&board_dat->data->lock);
+	for (i = 0; i < board_dat->num; i++) {
+		count = 500;
+		spin_lock(&board_dat->data[i]->lock);
+		board_dat->data[i]->status = STATUS_EXITING;
+		while ((list_empty(&board_dat->data[i]->queue) == 0) && --count) {
+			dev_dbg(&board_dat->pdev->dev, "%s :queue not empty\n",
+				__func__);
+			spin_unlock(&board_dat->data[i]->lock);
+			msleep(PCH_SLEEP_TIME);
+			spin_lock(&board_dat->data[i]->lock);
+		}
+		spin_unlock(&board_dat->data[i]->lock);
 	}
-	spin_unlock(&board_dat->data->lock);
 
 	/* Free resources allocated for PCH SPI */
 	pch_spi_free_resources(board_dat);
 
-	spi_unregister_master(board_dat->data->master);
+	/* Unregister SPI master */
+	for (i = 0; i < board_dat->num; i++)
+		spi_unregister_master(board_dat->data[i]->master);
 
 	/* free memory for private data */
 	kfree(board_dat);
@@ -1146,6 +1189,7 @@ static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	u8 count;
 	int retval;
+	int i;
 
 	struct pch_spi_board_data *board_dat = pci_get_drvdata(pdev);
 
@@ -1162,25 +1206,29 @@ static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state)
 
 	/* check if the current message is processed:
 	   Only after thats done the transfer will be suspended */
-	count = 255;
-	while ((--count) > 0) {
-		if (!(board_dat->data->bcurrent_msg_processing)) {
-			dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_"
-				"msg_processing = false\n", __func__);
-			break;
-		} else {
-			dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_msg_"
-				"processing = true\n", __func__);
+	for (i = 0; i < board_dat->num; i++) {
+		count = 255;
+		while ((--count) > 0) {
+			if (!(board_dat->data[i]->bcurrent_msg_processing)) {
+				dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_"
+					"msg_processing = false\n", __func__);
+				break;
+			} else {
+				dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_msg_"
+					"processing = true\n", __func__);
+			}
+			msleep(PCH_SLEEP_TIME);
 		}
-		msleep(PCH_SLEEP_TIME);
 	}
 
 	/* Free IRQ */
 	if (board_dat->irq_reg_sts) {
-		/* disable all interrupts */
-		pch_spi_setclr_reg(board_dat->data->master, PCH_SPCR, 0,
-				   PCH_ALL);
-		pch_spi_reset(board_dat->data->master);
+		/* disable all interrupts */	
+		for (i = 0; i < board_dat->num; i++) {
+			pch_spi_setclr_reg(board_dat->data[i]->master, PCH_SPCR, 0,
+					   PCH_ALL);
+			pch_spi_reset(board_dat->data[i]->master);
+		}
 
 		free_irq(board_dat->pdev->irq, board_dat);
 
@@ -1221,6 +1269,7 @@ static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state)
 static int pch_spi_resume(struct pci_dev *pdev)
 {
 	int retval;
+	int i;
 
 	struct pch_spi_board_data *board = pci_get_drvdata(pdev);
 	dev_dbg(&pdev->dev, "%s ENTRY\n", __func__);
@@ -1259,8 +1308,10 @@ static int pch_spi_resume(struct pci_dev *pdev)
 			board->irq_reg_sts = true;
 
 			/* reset PCH SPI h/w */
-			pch_spi_reset(board->data->master);
-			pch_spi_set_master_mode(board->data->master);
+			for (i = 0; i < board->num; i++) {
+				pch_spi_reset(board->data[i]->master);
+				pch_spi_set_master_mode(board->data[i]->master);
+			}
 
 			/* set suspend status to false */
 			board->suspend_sts = false;
-- 
1.6.0.6

[PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove

[Tomoya MORINAGA]

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/spi/spi_topcliff_pch.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi_topcliff_pch.c b/drivers/spi/spi_topcliff_pch.c
index 18e077b..a796eaf 100644
--- a/drivers/spi/spi_topcliff_pch.c
+++ b/drivers/spi/spi_topcliff_pch.c
@@ -1166,21 +1166,21 @@ static void pch_spi_remove(struct pci_dev *pdev)
 		spin_unlock(&board_dat->data[i]->lock);
 	}
 
-	/* Free resources allocated for PCH SPI */
-	pch_spi_free_resources(board_dat);
-
 	/* Unregister SPI master */
 	for (i = 0; i < board_dat->num; i++)
 		spi_unregister_master(board_dat->data[i]->master);
 
-	/* free memory for private data */
-	kfree(board_dat);
+	/* Free resources allocated for PCH SPI */
+	pch_spi_free_resources(board_dat);
 
 	pci_set_drvdata(pdev, NULL);
 
 	/* disable PCI device */
 	pci_disable_device(pdev);
 
+	/* free memory for private data */
+	kfree(board_dat);
+
 	dev_dbg(&pdev->dev, "%s invoked pci_disable_device\n", __func__);
 }
 
-- 
1.6.0.6

[PATCH 3/3] spi_topcliff_pch: fix resource leak issue

[Tomoya MORINAGA]

In case spi_register_master fails in probe function,
pch_spi_free_resources is not called.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/spi/spi_topcliff_pch.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi_topcliff_pch.c b/drivers/spi/spi_topcliff_pch.c
index a796eaf..e806e31 100644
--- a/drivers/spi/spi_topcliff_pch.c
+++ b/drivers/spi/spi_topcliff_pch.c
@@ -1125,6 +1125,7 @@ static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 err_spi_reg_master:
 	for (j = 0; j < i; j++)
 		spi_unregister_master(master[j]);
+	pch_spi_free_resources(board_dat);
 err_spi_get_resources:
 err_spi_alloc_master:
 	for (j = 0; j < i; j++)
-- 
1.6.0.6

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Grant Likely]

On Mon, Dec 27, 2010 at 08:23:45PM +0900, Tomoya MORINAGA wrote:
> Support ML7213 device of OKI SEMICONDUCTOR.
> ML7213 is companion chip of Intel Atom E6xx series for IVI(In-Vehicle Infotainment).
> ML7213 is completely compatible for Intel EG20T PCH.
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
> ---
>  drivers/spi/spi_topcliff_pch.c |  293 +++++++++++++++++++++++-----------------

Hi Tomoya,

As previously discussed on this list, I would like to see support for
multiple bus instances implemented differently.  Rather than storing
the spi_master instances in an array in the pci device private data,
the pci device should register a separate platform_device for each spi
bus instance, and each of those bus instances should get bound to a
topcliff_spi_bus driver which doesn't need to have any special
knowledge about how many spi_master instances actually exist.

Basically, the way it is implemented in this patch isn't taking
advantage of the infrastructure and instance management provided by
the driver model.

g.

>  1 files changed, 172 insertions(+), 121 deletions(-)
> 
> diff --git a/drivers/spi/spi_topcliff_pch.c b/drivers/spi/spi_topcliff_pch.c
> index 58e187f..18e077b 100644
> --- a/drivers/spi/spi_topcliff_pch.c
> +++ b/drivers/spi/spi_topcliff_pch.c
> @@ -35,6 +35,7 @@
>  #define PCH_SPDRR		0x10	/* SPI read data register */
>  #define PCH_SSNXCR		0x18	/* SSN Expand Control Register */
>  #define PCH_SRST		0x1C	/* SPI reset register */
> +#define PCH_SPI_ADDRESS_SIZE	0x20
>  
>  #define PCH_SPSR_TFD		0x000007C0
>  #define PCH_SPSR_RFD		0x0000F800
> @@ -88,6 +89,16 @@
>  #define PCH_CLOCK_HZ		50000000
>  #define PCH_MAX_SPBR		1023
>  
> +/* Definition for ML7213 by OKI SEMICONDUCTOR */
> +#define PCI_VENDOR_ID_ROHM		0x10DB
> +#define PCI_DEVICE_ID_ML7213_SPI	0x802c
> +
> +/*
> +Set the number of SPI instance max
> +Intel EG20T PCH :		1ch
> +OKI SEMICONDUCTOR ML7213 IOH :	2ch
> +*/
> +#define PCH_SPI_MAX_DEV			2
>  
>  /**
>   * struct pch_spi_data - Holds the SPI channel specific details
> @@ -153,18 +164,21 @@ struct pch_spi_data {
>   * @pci_req_sts:	Status of pci_request_regions
>   * @suspend_sts:	Status of suspend
>   * @data:		Pointer to SPI channel data structure
> + * @num:		The number of SPI device instance
>   */
>  struct pch_spi_board_data {
>  	struct pci_dev *pdev;
>  	u8 irq_reg_sts;
>  	u8 pci_req_sts;
>  	u8 suspend_sts;
> -	struct pch_spi_data *data;
> +	struct pch_spi_data *data[PCH_SPI_MAX_DEV];
> +	int num;
>  };
>  
>  static struct pci_device_id pch_spi_pcidev_id[] = {
> -	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_GE_SPI)},
> -	{0,}
> +	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_GE_SPI),    1, },
> +	{ PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_SPI), 2, },
> +	{ }
>  };
>  
>  /**
> @@ -267,7 +281,7 @@ static void pch_spi_handler_sub(struct pch_spi_data *data, u32 reg_spsr_val,
>  	if (reg_spsr_val & SPSR_FI_BIT) {
>  		/* disable FI & RFI interrupts */
>  		pch_spi_setclr_reg(data->master, PCH_SPCR, 0,
> -				   SPCR_FIE_BIT | SPCR_TFIE_BIT);
> +				   SPCR_FIE_BIT | SPCR_RFIE_BIT);
>  
>  		/* transfer is completed;inform pch_spi_process_messages */
>  		data->transfer_complete = true;
> @@ -288,6 +302,7 @@ static irqreturn_t pch_spi_handler(int irq, void *dev_id)
>  	void __iomem *io_remap_addr;
>  	irqreturn_t ret = IRQ_NONE;
>  	struct pch_spi_board_data *board_dat = dev_id;
> +	int i;
>  
>  	if (board_dat->suspend_sts) {
>  		dev_dbg(&board_dat->pdev->dev,
> @@ -295,21 +310,22 @@ static irqreturn_t pch_spi_handler(int irq, void *dev_id)
>  		return IRQ_NONE;
>  	}
>  
> -	data = board_dat->data;
> -	io_remap_addr = data->io_remap_addr;
> -	spsr = io_remap_addr + PCH_SPSR;
> +	for (i = 0; i < board_dat->num; i++) {
> +		data = board_dat->data[i];
> +		io_remap_addr = data->io_remap_addr;
> +		spsr = io_remap_addr + PCH_SPSR;
>  
> -	reg_spsr_val = ioread32(spsr);
> +		reg_spsr_val = ioread32(spsr);
>  
> -	/* Check if the interrupt is for SPI device */
> -	if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
> -		pch_spi_handler_sub(data, reg_spsr_val, io_remap_addr);
> -		ret = IRQ_HANDLED;
> -	}
> -
> -	dev_dbg(&board_dat->pdev->dev, "%s EXIT return value=%d\n",
> -		__func__, ret);
> +		/* Check if the interrupt is for SPI device */
> +		if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
> +			pch_spi_handler_sub(data, reg_spsr_val, io_remap_addr);
> +			ret = IRQ_HANDLED;
> +		}
>  
> +		dev_dbg(&board_dat->pdev->dev, "%s EXIT return value=%d\n",
> +			__func__, ret);
> +	}
>  	return ret;
>  }
>  
> @@ -679,11 +695,11 @@ static void pch_spi_set_ir(struct pch_spi_data *data)
>  	if ((data->bpw_len) > PCH_MAX_FIFO_DEPTH) {
>  		/* set receive threhold to PCH_RX_THOLD */
>  		pch_spi_setclr_reg(data->master, PCH_SPCR,
> -				   PCH_RX_THOLD << SPCR_TFIC_FIELD,
> -				   ~MASK_TFIC_SPCR_BITS);
> +				   PCH_RX_THOLD << SPCR_RFIC_FIELD,
> +				   ~MASK_RFIC_SPCR_BITS);
>  		/* enable FI and RFI interrupts */
>  		pch_spi_setclr_reg(data->master, PCH_SPCR,
> -				   SPCR_RFIE_BIT | SPCR_TFIE_BIT, 0);
> +				   SPCR_RFIE_BIT | SPCR_FIE_BIT, 0);
>  	} else {
>  		/* set receive threhold to maximum */
>  		pch_spi_setclr_reg(data->master, PCH_SPCR,
> @@ -870,37 +886,46 @@ static void pch_spi_process_messages(struct work_struct *pwork)
>  
>  static void pch_spi_free_resources(struct pch_spi_board_data *board_dat)
>  {
> +	int i;
> +
>  	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);
>  
>  	/* free workqueue */
> -	if (board_dat->data->wk != NULL) {
> -		destroy_workqueue(board_dat->data->wk);
> -		board_dat->data->wk = NULL;
> -		dev_dbg(&board_dat->pdev->dev,
> -			"%s destroy_workqueue invoked successfully\n",
> -			__func__);
> +	for (i = 0; i < board_dat->num; i++) {
> +		if (board_dat->data[i]->wk != NULL) {
> +			destroy_workqueue(board_dat->data[i]->wk);
> +			board_dat->data[i]->wk = NULL;
> +			dev_dbg(&board_dat->pdev->dev,
> +				"%s destroy_workqueue invoked successfully\n",
> +				__func__);
> +		}
>  	}
>  
>  	/* disable interrupts & free IRQ */
>  	if (board_dat->irq_reg_sts) {
> -		/* disable interrupts */
> -		pch_spi_setclr_reg(board_dat->data->master, PCH_SPCR, 0,
> -				   PCH_ALL);
> +		for (i = 0; i < board_dat->num; i++) {
> +			/* disable interrupts */
> +			pch_spi_setclr_reg(board_dat->data[i]->master, PCH_SPCR, 0,
> +					   PCH_ALL);
>  
> -		/* free IRQ */
> -		free_irq(board_dat->pdev->irq, board_dat);
> +			if (i == (board_dat->num - 1)) {
> +				/* free IRQ */
> +				free_irq(board_dat->pdev->irq, board_dat);
>  
> -		dev_dbg(&board_dat->pdev->dev,
> -			"%s free_irq invoked successfully\n", __func__);
> +				dev_dbg(&board_dat->pdev->dev,
> +					"%s free_irq invoked successfully\n", __func__);
>  
> -		board_dat->irq_reg_sts = false;
> +				board_dat->irq_reg_sts = false;
> +			}
> +		}
>  	}
>  
>  	/* unmap PCI base address */
> -	if (board_dat->data->io_remap_addr != 0) {
> -		pci_iounmap(board_dat->pdev, board_dat->data->io_remap_addr);
> +	if (board_dat->data[0]->io_remap_addr != 0) {
> +		pci_iounmap(board_dat->pdev, board_dat->data[0]->io_remap_addr);
>  
> -		board_dat->data->io_remap_addr = 0;
> +		for (i = 0; i < board_dat->num; i++)
> +			board_dat->data[i]->io_remap_addr = 0;
>  
>  		dev_dbg(&board_dat->pdev->dev,
>  			"%s pci_iounmap invoked successfully\n", __func__);
> @@ -920,19 +945,23 @@ static int pch_spi_get_resources(struct pch_spi_board_data *board_dat)
>  {
>  	void __iomem *io_remap_addr;
>  	int retval;
> +	int i;
> +
>  	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);
>  
> -	/* create workqueue */
> -	board_dat->data->wk = create_singlethread_workqueue(KBUILD_MODNAME);
> -	if (!board_dat->data->wk) {
> -		dev_err(&board_dat->pdev->dev,
> -			"%s create_singlet hread_workqueue failed\n", __func__);
> -		retval = -EBUSY;
> -		goto err_return;
> -	}
> +	for (i = 0; i < board_dat->num; i++) {
> +		/* create workqueue */
> +		board_dat->data[i]->wk = create_singlethread_workqueue(KBUILD_MODNAME);
> +		if (!board_dat->data[i]->wk) {
> +			dev_err(&board_dat->pdev->dev,
> +				"%s create_singlet hread_workqueue failed\n", __func__);
> +			retval = -EBUSY;
> +			goto err_return;
> +		}
>  
> -	dev_dbg(&board_dat->pdev->dev,
> -		"%s create_singlethread_workqueue success\n", __func__);
> +		dev_dbg(&board_dat->pdev->dev,
> +			"%s create_singlethread_workqueue success\n", __func__);
> +	}
>  
>  	retval = pci_request_regions(board_dat->pdev, KBUILD_MODNAME);
>  	if (retval != 0) {
> @@ -951,13 +980,15 @@ static int pch_spi_get_resources(struct pch_spi_board_data *board_dat)
>  		goto err_return;
>  	}
>  
> -	/* calculate base address for all channels */
> -	board_dat->data->io_remap_addr = io_remap_addr;
> +	for (i = 0; i < board_dat->num; i++) {
> +		/* calculate base address for all channels */
> +		board_dat->data[i]->io_remap_addr = io_remap_addr + (PCH_SPI_ADDRESS_SIZE * i);
>  
> -	/* reset PCH SPI h/w */
> -	pch_spi_reset(board_dat->data->master);
> -	dev_dbg(&board_dat->pdev->dev,
> -		"%s pch_spi_reset invoked successfully\n", __func__);
> +		/* reset PCH SPI h/w */
> +		pch_spi_reset(board_dat->data[i]->master);
> +		dev_dbg(&board_dat->pdev->dev,
> +			"%s pch_spi_reset invoked successfully\n", __func__);
> +	}
>  
>  	/* register IRQ */
>  	retval = request_irq(board_dat->pdev->irq, pch_spi_handler,
> @@ -989,10 +1020,11 @@ err_return:
>  static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  
> -	struct spi_master *master;
> +	struct spi_master *master[PCH_SPI_MAX_DEV];
>  
>  	struct pch_spi_board_data *board_dat;
>  	int retval;
> +	int i, j;
>  
>  	dev_dbg(&pdev->dev, "%s ENTRY\n", __func__);
>  
> @@ -1021,37 +1053,40 @@ static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		__func__, retval);
>  
>  	board_dat->pdev = pdev;
> +	board_dat->num = id->driver_data;
> +
> +	for (i = 0; i < board_dat->num; i++) {
> +		/* alllocate memory for SPI master */
> +		master[i] = spi_alloc_master(&pdev->dev, sizeof(struct pch_spi_data));
> +		if (master[i] == NULL) {
> +			retval = -ENOMEM;
> +			dev_err(&pdev->dev, "%s Fail.\n", __func__);
> +			goto err_spi_alloc_master;
> +		}
>  
> -	/* alllocate memory for SPI master */
> -	master = spi_alloc_master(&pdev->dev, sizeof(struct pch_spi_data));
> -	if (master == NULL) {
> -		retval = -ENOMEM;
> -		dev_err(&pdev->dev, "%s Fail.\n", __func__);
> -		goto err_spi_alloc_master;
> -	}
> -
> -	dev_dbg(&pdev->dev,
> -		"%s spi_alloc_master returned non NULL\n", __func__);
> +		dev_dbg(&pdev->dev,
> +			"%s spi_alloc_master returned non NULL\n", __func__);
>  
> -	/* initialize members of SPI master */
> -	master->bus_num = -1;
> -	master->num_chipselect = PCH_MAX_CS;
> -	master->setup = pch_spi_setup;
> -	master->transfer = pch_spi_transfer;
> -	dev_dbg(&pdev->dev,
> -		"%s transfer member of SPI master initialized\n", __func__);
> +		/* initialize members of SPI master */
> +		master[i]->bus_num = -1;
> +		master[i]->num_chipselect = PCH_MAX_CS;
> +		master[i]->setup = pch_spi_setup;
> +		master[i]->transfer = pch_spi_transfer;
> +		dev_dbg(&pdev->dev,
> +			"%s transfer member of SPI master initialized\n", __func__);
>  
> -	board_dat->data = spi_master_get_devdata(master);
> +		board_dat->data[i] = spi_master_get_devdata(master[i]);
>  
> -	board_dat->data->master = master;
> -	board_dat->data->n_curnt_chip = 255;
> -	board_dat->data->board_dat = board_dat;
> -	board_dat->data->status = STATUS_RUNNING;
> +		board_dat->data[i]->master = master[i];
> +		board_dat->data[i]->n_curnt_chip = 255;
> +		board_dat->data[i]->board_dat = board_dat;
> +		board_dat->data[i]->status = STATUS_RUNNING;
>  
> -	INIT_LIST_HEAD(&board_dat->data->queue);
> -	spin_lock_init(&board_dat->data->lock);
> -	INIT_WORK(&board_dat->data->work, pch_spi_process_messages);
> -	init_waitqueue_head(&board_dat->data->wait);
> +		INIT_LIST_HEAD(&board_dat->data[i]->queue);
> +		spin_lock_init(&board_dat->data[i]->lock);
> +		INIT_WORK(&board_dat->data[i]->work, pch_spi_process_messages);
> +		init_waitqueue_head(&board_dat->data[i]->wait);
> +	}
>  
>  	/* allocate resources for PCH SPI */
>  	retval = pch_spi_get_resources(board_dat);
> @@ -1068,29 +1103,32 @@ static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	dev_dbg(&pdev->dev, "%s invoked pci_set_drvdata\n", __func__);
>  
>  	/* set master mode */
> -	pch_spi_set_master_mode(master);
> -	dev_dbg(&pdev->dev,
> -		"%s invoked pch_spi_set_master_mode\n", __func__);
> +	for (i = 0; i < board_dat->num; i++) {
> +		pch_spi_set_master_mode(master[i]);
> +		dev_dbg(&pdev->dev,
> +			"%s invoked pch_spi_set_master_mode\n", __func__);
> +
> +		/* Register the controller with the SPI core. */
> +		retval = spi_register_master(master[i]);
> +		if (retval != 0) {
> +			dev_err(&pdev->dev,
> +				"%s spi_register_master FAILED\n", __func__);
> +			goto err_spi_reg_master;
> +		}
>  
> -	/* Register the controller with the SPI core. */
> -	retval = spi_register_master(master);
> -	if (retval != 0) {
> -		dev_err(&pdev->dev,
> -			"%s spi_register_master FAILED\n", __func__);
> -		goto err_spi_reg_master;
> +		dev_dbg(&pdev->dev, "%s spi_register_master returned=%d\n",
> +			__func__, retval);
>  	}
>  
> -	dev_dbg(&pdev->dev, "%s spi_register_master returned=%d\n",
> -		__func__, retval);
> -
> -
>  	return 0;
>  
>  err_spi_reg_master:
> -	spi_unregister_master(master);
> +	for (j = 0; j < i; j++)
> +		spi_unregister_master(master[j]);
>  err_spi_get_resources:
>  err_spi_alloc_master:
> -	spi_master_put(master);
> +	for (j = 0; j < i; j++)
> +		spi_master_put(master[j]);
>  	pci_disable_device(pdev);
>  err_pci_en_device:
>  	kfree(board_dat);
> @@ -1102,6 +1140,7 @@ static void pch_spi_remove(struct pci_dev *pdev)
>  {
>  	struct pch_spi_board_data *board_dat = pci_get_drvdata(pdev);
>  	int count;
> +	int i;
>  
>  	dev_dbg(&pdev->dev, "%s ENTRY\n", __func__);
>  
> @@ -1113,22 +1152,26 @@ static void pch_spi_remove(struct pci_dev *pdev)
>  
>  	/* check for any pending messages; no action is taken if the queue
>  	 * is still full; but at least we tried.  Unload anyway */
> -	count = 500;
> -	spin_lock(&board_dat->data->lock);
> -	board_dat->data->status = STATUS_EXITING;
> -	while ((list_empty(&board_dat->data->queue) == 0) && --count) {
> -		dev_dbg(&board_dat->pdev->dev, "%s :queue not empty\n",
> -			__func__);
> -		spin_unlock(&board_dat->data->lock);
> -		msleep(PCH_SLEEP_TIME);
> -		spin_lock(&board_dat->data->lock);
> +	for (i = 0; i < board_dat->num; i++) {
> +		count = 500;
> +		spin_lock(&board_dat->data[i]->lock);
> +		board_dat->data[i]->status = STATUS_EXITING;
> +		while ((list_empty(&board_dat->data[i]->queue) == 0) && --count) {
> +			dev_dbg(&board_dat->pdev->dev, "%s :queue not empty\n",
> +				__func__);
> +			spin_unlock(&board_dat->data[i]->lock);
> +			msleep(PCH_SLEEP_TIME);
> +			spin_lock(&board_dat->data[i]->lock);
> +		}
> +		spin_unlock(&board_dat->data[i]->lock);
>  	}
> -	spin_unlock(&board_dat->data->lock);
>  
>  	/* Free resources allocated for PCH SPI */
>  	pch_spi_free_resources(board_dat);
>  
> -	spi_unregister_master(board_dat->data->master);
> +	/* Unregister SPI master */
> +	for (i = 0; i < board_dat->num; i++)
> +		spi_unregister_master(board_dat->data[i]->master);
>  
>  	/* free memory for private data */
>  	kfree(board_dat);
> @@ -1146,6 +1189,7 @@ static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state)
>  {
>  	u8 count;
>  	int retval;
> +	int i;
>  
>  	struct pch_spi_board_data *board_dat = pci_get_drvdata(pdev);
>  
> @@ -1162,25 +1206,29 @@ static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state)
>  
>  	/* check if the current message is processed:
>  	   Only after thats done the transfer will be suspended */
> -	count = 255;
> -	while ((--count) > 0) {
> -		if (!(board_dat->data->bcurrent_msg_processing)) {
> -			dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_"
> -				"msg_processing = false\n", __func__);
> -			break;
> -		} else {
> -			dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_msg_"
> -				"processing = true\n", __func__);
> +	for (i = 0; i < board_dat->num; i++) {
> +		count = 255;
> +		while ((--count) > 0) {
> +			if (!(board_dat->data[i]->bcurrent_msg_processing)) {
> +				dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_"
> +					"msg_processing = false\n", __func__);
> +				break;
> +			} else {
> +				dev_dbg(&pdev->dev, "%s board_dat->data->bCurrent_msg_"
> +					"processing = true\n", __func__);
> +			}
> +			msleep(PCH_SLEEP_TIME);
>  		}
> -		msleep(PCH_SLEEP_TIME);
>  	}
>  
>  	/* Free IRQ */
>  	if (board_dat->irq_reg_sts) {
> -		/* disable all interrupts */
> -		pch_spi_setclr_reg(board_dat->data->master, PCH_SPCR, 0,
> -				   PCH_ALL);
> -		pch_spi_reset(board_dat->data->master);
> +		/* disable all interrupts */	
> +		for (i = 0; i < board_dat->num; i++) {
> +			pch_spi_setclr_reg(board_dat->data[i]->master, PCH_SPCR, 0,
> +					   PCH_ALL);
> +			pch_spi_reset(board_dat->data[i]->master);
> +		}
>  
>  		free_irq(board_dat->pdev->irq, board_dat);
>  
> @@ -1221,6 +1269,7 @@ static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state)
>  static int pch_spi_resume(struct pci_dev *pdev)
>  {
>  	int retval;
> +	int i;
>  
>  	struct pch_spi_board_data *board = pci_get_drvdata(pdev);
>  	dev_dbg(&pdev->dev, "%s ENTRY\n", __func__);
> @@ -1259,8 +1308,10 @@ static int pch_spi_resume(struct pci_dev *pdev)
>  			board->irq_reg_sts = true;
>  
>  			/* reset PCH SPI h/w */
> -			pch_spi_reset(board->data->master);
> -			pch_spi_set_master_mode(board->data->master);
> +			for (i = 0; i < board->num; i++) {
> +				pch_spi_reset(board->data[i]->master);
> +				pch_spi_set_master_mode(board->data[i]->master);
> +			}
>  
>  			/* set suspend status to false */
>  			board->suspend_sts = false;
> -- 
> 1.6.0.6
> 

Re: [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove

[Grant Likely]

On Mon, Dec 27, 2010 at 08:23:46PM +0900, Tomoya MORINAGA wrote:
> Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>

Hi Tomoya,

This looks like a bug fix that should be applied now, but I cannot
apply it to mainline because it depends on the first patch in your
series which I'm not going to apply.  If you respin this patch to
apply without the first patch, then I can pick it up immediately.

g.

> ---
>  drivers/spi/spi_topcliff_pch.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/spi/spi_topcliff_pch.c b/drivers/spi/spi_topcliff_pch.c
> index 18e077b..a796eaf 100644
> --- a/drivers/spi/spi_topcliff_pch.c
> +++ b/drivers/spi/spi_topcliff_pch.c
> @@ -1166,21 +1166,21 @@ static void pch_spi_remove(struct pci_dev *pdev)
>  		spin_unlock(&board_dat->data[i]->lock);
>  	}
>  
> -	/* Free resources allocated for PCH SPI */
> -	pch_spi_free_resources(board_dat);
> -
>  	/* Unregister SPI master */
>  	for (i = 0; i < board_dat->num; i++)
>  		spi_unregister_master(board_dat->data[i]->master);
>  
> -	/* free memory for private data */
> -	kfree(board_dat);
> +	/* Free resources allocated for PCH SPI */
> +	pch_spi_free_resources(board_dat);
>  
>  	pci_set_drvdata(pdev, NULL);
>  
>  	/* disable PCI device */
>  	pci_disable_device(pdev);
>  
> +	/* free memory for private data */
> +	kfree(board_dat);
> +
>  	dev_dbg(&pdev->dev, "%s invoked pci_disable_device\n", __func__);
>  }
>  
> -- 
> 1.6.0.6
> 

Re: [PATCH 3/3] spi_topcliff_pch: fix resource leak issue

[Grant Likely]

On Mon, Dec 27, 2010 at 08:23:47PM +0900, Tomoya MORINAGA wrote:
> In case spi_register_master fails in probe function,
> pch_spi_free_resources is not called.
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>

Ditto for patch 2.  This looks like a bug fix, please respin to apply
without patch #1

Thanks,
g.

> ---
>  drivers/spi/spi_topcliff_pch.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/spi/spi_topcliff_pch.c b/drivers/spi/spi_topcliff_pch.c
> index a796eaf..e806e31 100644
> --- a/drivers/spi/spi_topcliff_pch.c
> +++ b/drivers/spi/spi_topcliff_pch.c
> @@ -1125,6 +1125,7 @@ static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  err_spi_reg_master:
>  	for (j = 0; j < i; j++)
>  		spi_unregister_master(master[j]);
> +	pch_spi_free_resources(board_dat);
>  err_spi_get_resources:
>  err_spi_alloc_master:
>  	for (j = 0; j < i; j++)
> -- 
> 1.6.0.6
> 

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Grant,

On Wednesday, December 29, 2010 3:49 PM, Grant Likely wrote:
> As previously discussed on this list, I would like to see support for
> multiple bus instances implemented differently.  Rather than storing
> the spi_master instances in an array in the pci device private data,
> the pci device should register a separate platform_device for each spi
> bus instance, and each of those bus instances should get bound to a
> topcliff_spi_bus driver which doesn't need to have any special
> knowledge about how many spi_master instances actually exist.
> 
> Basically, the way it is implemented in this patch isn't taking
> advantage of the infrastructure and instance management provided by
> the driver model.

I see.
I will modify like your suggestion.
Could you show the reference SPI driver using multi-instance ?

Thanks,
---
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Grant Likely]

On Wed, Jan 05, 2011 at 11:34:20AM +0900, Tomoya MORINAGA wrote:
> Hi Grant,
> 
> On Wednesday, December 29, 2010 3:49 PM, Grant Likely wrote:
> >As previously discussed on this list, I would like to see support for
> >multiple bus instances implemented differently.  Rather than storing
> >the spi_master instances in an array in the pci device private data,
> >the pci device should register a separate platform_device for each spi
> >bus instance, and each of those bus instances should get bound to a
> >topcliff_spi_bus driver which doesn't need to have any special
> >knowledge about how many spi_master instances actually exist.
> >
> >Basically, the way it is implemented in this patch isn't taking
> >advantage of the infrastructure and instance management provided by
> >the driver model.
> 
> I see.
> I will modify like your suggestion.
> Could you show the reference SPI driver using multi-instance ?

In an multi-instance scenario, the spi driver is just a plain-jane
platform_driver.  There isn't anything special about it.  Instead, the
pci_driver becomes responsible to register one or more platform
devices for the child busses.  Much like one of the multifunction
devices in drivers/mfd.

As far as examples go, drivers/mfd/timberdale.c seems reasonable, if a
bit larger than what you need.

g.

> 
> Thanks,
> ---
> Tomoya MORINAGA
> OKI SEMICONDUCTOR CO., LTD.
> 

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Grant,

On Thursday, January 06, 2011 1:39 AM, Grant Likely wrote:
> As far as examples go, drivers/mfd/timberdale.c seems 
> reasonable, if a bit larger than what you need.

Thank you for your suggestion.
However, though seeing this driver, I didn't understand your intention.
It seems platform_device/platform_driver are not used in this.
Which part of this driver do you want to show me as reference ?

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

> -----Original Message-----
> From: Grant Likely [mailto:glikely@secretlab.ca] On Behalf Of 
> Grant Likely
> Sent: Thursday, January 06, 2011 1:39 AM
> To: Tomoya MORINAGA
> Cc: David Brownell; spi-devel-general@lists.sourceforge.net; 
> linux-kernel@vger.kernel.org; qi.wang@intel.com; 
> yong.y.wang@intel.com; joel.clark@intel.com; 
> kok.howg.ewe@intel.com; Toshiharu Okada
> Subject: Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213
> 
> On Wed, Jan 05, 2011 at 11:34:20AM +0900, Tomoya MORINAGA wrote:
> > Hi Grant,
> > 
> > On Wednesday, December 29, 2010 3:49 PM, Grant Likely wrote:
> > >As previously discussed on this list, I would like to see 
> support for 
> > >multiple bus instances implemented differently.  Rather 
> than storing 
> > >the spi_master instances in an array in the pci device 
> private data, 
> > >the pci device should register a separate platform_device for each 
> > >spi bus instance, and each of those bus instances should 
> get bound to 
> > >a topcliff_spi_bus driver which doesn't need to have any special 
> > >knowledge about how many spi_master instances actually exist.
> > >
> > >Basically, the way it is implemented in this patch isn't taking 
> > >advantage of the infrastructure and instance management 
> provided by 
> > >the driver model.
> > 
> > I see.
> > I will modify like your suggestion.
> > Could you show the reference SPI driver using multi-instance ?
> 
> In an multi-instance scenario, the spi driver is just a 
> plain-jane platform_driver.  There isn't anything special 
> about it.  Instead, the pci_driver becomes responsible to 
> register one or more platform devices for the child busses.  
> Much like one of the multifunction devices in drivers/mfd.
> 
> As far as examples go, drivers/mfd/timberdale.c seems 
> reasonable, if a bit larger than what you need.
> 
> g.
> 
> > 
> > Thanks,
> > ---
> > Tomoya MORINAGA
> > OKI SEMICONDUCTOR CO., LTD.
> > 
> 

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Grant,

Sorry, I made mistake in my-mail address.
I sent again.

On Thursday, January 06, 2011 1:39 AM, Grant Likely wrote:
> As far as examples go, drivers/mfd/timberdale.c seems reasonable, if a 
> bit larger than what you need.

Thank you for your suggestion.
However, though seeing this driver, I didn't understand your intention.
It seems platform_device/platform_driver are not used in this.
Which part of this driver do you want to show me as reference ?

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Milton Miller]

On: Mon, 27 Dec 2010 at about 11:23:45 -0000, Tomoya MORINAGA wrote:
> On Thursday, January 06, 2011 1:39 AM, Grant Likely wrote:
> > As far as examples go, drivers/mfd/timberdale.c seems reasonable, if a 
> > bit larger than what you need.
> 
> Thank you for your suggestion.
> However, though seeing this driver, I didn't understand your intention.
> It seems platform_device/platform_driver are not used in this.
> Which part of this driver do you want to show me as reference ?

follow mfd_add_devices into drivers/mfd/mfd-core.c

milton

------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web.   Learn how to 
best implement a security strategy that keeps consumers' information secure 
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl 

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Milton Miller]

On: Mon, 27 Dec 2010 at about 11:23:45 -0000, Tomoya MORINAGA wrote:
> On Thursday, January 06, 2011 1:39 AM, Grant Likely wrote:
> > As far as examples go, drivers/mfd/timberdale.c seems reasonable, if a 
> > bit larger than what you need.
> 
> Thank you for your suggestion.
> However, though seeing this driver, I didn't understand your intention.
> It seems platform_device/platform_driver are not used in this.
> Which part of this driver do you want to show me as reference ?

follow mfd_add_devices into drivers/mfd/mfd-core.c

milton

------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web.   Learn how to 
best implement a security strategy that keeps consumers' information secure 
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl 

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Milton,

On Friday, January 07, 2011 6:22 PM, Milton Miller wrote:
> follow mfd_add_devices into drivers/mfd/mfd-core.c

It seems mfd_add_devices/mfd_add_device are valuable for us.
I'll study this.

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.


------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web.   Learn how to 
best implement a security strategy that keeps consumers' information secure 
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl 

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Milton Miller]

On: Mon, 27 Dec 2010 at about 11:23:45 -0000, Tomoya MORINAGA wrote:
> On Thursday, January 06, 2011 1:39 AM, Grant Likely wrote:
> > As far as examples go, drivers/mfd/timberdale.c seems reasonable, if a 
> > bit larger than what you need.
> 
> Thank you for your suggestion.
> However, though seeing this driver, I didn't understand your intention.
> It seems platform_device/platform_driver are not used in this.
> Which part of this driver do you want to show me as reference ?

follow mfd_add_devices into drivers/mfd/mfd-core.c

milton

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Grant,

On Friday, January 07, 2011 6:22 PM, Milton Miller wrote:
> follow mfd_add_devices into drivers/mfd/mfd-core.c
I have also seen mfd-core.c.
However, I couldn't clear my question.

Q1. It seems platform_device is not used in your example. Does need "platform_device" ?
Q2. How detect pch_spi channel dynamically ?

I want to catch your saying correctly.
Could you give me more information ?

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.


------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web.   Learn how to 
best implement a security strategy that keeps consumers' information secure 
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl 

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Resend
(Previous destination address is broken partly.)

Hi Grant,

On Friday, January 07, 2011 6:22 PM, Milton Miller wrote:
> follow mfd_add_devices into drivers/mfd/mfd-core.c
I have also seen mfd-core.c.
However, I couldn't clear my question.

Q1. It seems platform_device is not used in your example. 
	Does need "platform_device" ?
Q2. How detect pch_spi channel dynamically ?

I want to catch your saying correctly.
Could you give me more information ?

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

On Tuesday, January 11, 2011 6:42 PM, Tomoya MORINAGA wrote:
> However, I couldn't clear my question.

I show the current our coding image.
Is the following agreeable with your saying ?

struct spi_platform_data {
	u16 num_chipselect;
	bool little_endian;
	u8 bits_per_word;
	struct spi_board_info *devices;
	u8 num_devices;
};

static struct resource pch_spi_resources[] = {
	{
		.start 	= 0,
		.end	= 0,
		.flags	= IORESOURCE_MEM,
	},
	{
		.start	= 0,
		.end	= 0,
		.flags	= IORESOURCE_IRQ,
	},
};

static struct spi_platform_data pch_spi_platform_data = {
	.num_chipselect = 2,
	.little_endian = true,
};

static struct pch_spi_cell pch_spi_cfg0 = {
	.name = "pch_spi",
	.num_resources = ARRAY_SIZE(pch_spi_resources),
	.resources = pch_spi_resources,
	.platform_data = &pch_spi_platform_data,
	.data_size = sizeof(pch_spi_platform_data),
};

static int pch_spi_get_resources(struct pch_spi_board_data *board_dat, int num)
{
	int retval;
	int i;

	/* create workqueue */
	board_dat->data[num]->wk = create_singlethread_workqueue(KBUILD_MODNAME);

	pci_request_regions(board_dat->pdev, KBUILD_MODNAME);
}

static int pch_spi_add_device(struct device *parent, int id,
			  struct pch_spi_cell *cell,
			  struct resource *mem_base, int irq_base,
			      struct pch_spi_board_data *board_dat)
{

	platform_device_alloc(cell->name, id + cell->id);

	res = kzalloc(sizeof(*res) * cell->num_resources, GFP_KERNEL);
	if (!res)
		goto fail_device;

	platform_set_drvdata(pdev, cell->driver_data);

	if (cell->data_size) {
		platform_device_add_data(pdev,
					cell->platform_data, cell->data_size);
	}

	for (i = 0; i < cell->num_resources; i++) {
		if ((cell->resources[i].flags & IORESOURCE_MEM) && mem_base) {
			res[i].parent = 
			res[i].start = 
			res[i].end = 
		} else if (cell->resources[i].flags & IORESOURCE_IRQ) {
			res[i].start = 
			res[i].end   = 
		} else {


	}

	ret = platform_device_add_resources(pdev, res, cell->num_resources);
	if (ret)
		goto fail_res;

	ret = platform_device_add(pdev);
	if (ret)
		goto fail_res;

	kfree(res);

	spi_alloc_master( );

	board_dat->data[num] = spi_master_get_devdata( );

	pch_spi_get_resources( );

	/* set master mode */
	pch_spi_set_master_mode( );

	/* Register the controller with the SPI core. */
	spi_register_master( );

	return 0;
}

int pch_spi_add_devices(struct device *parent,
		    struct pch_spi_cell *cells, int n_devs,
		    struct resource *mem_base,
		    struct pch_spi_board_data *board_dat)
{
	pci_iomap()

	for (i < n_devs)
		pch_spi_add_device()

}

static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
	board_dat = kzalloc(sizeof(struct pch_spi_board_data), GFP_KERNEL);
	pci_enable_device()
	request_irq()
	pch_spi_add_devices(&pdev->dev, 
			&pch_spi_cfg0,
			2,
			&pdev->resource[0], board_dat);

}

static irqreturn_t pch_spi_handler(int irq, void *dev_id)
{

	for (i = 0; i < board_dat->num; i++) {
		data = board_dat->data[i];
		io_remap_addr = data->io_remap_addr;
		spsr = io_remap_addr + PCH_SPSR;

		reg_spsr_val = ioread32(spsr);

		/* Check if the interrupt is for SPI device */
		if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
			pch_spi_handler_sub(data, reg_spsr_val, io_remap_addr);
			ret = IRQ_HANDLED;
		}

		dev_dbg(&board_dat->pdev->dev, "%s EXIT return value=%d\n",
			__func__, ret);
	}
	return ret;
}

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Grant,

Need your help!
Could you give us your answer ?

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.


> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org 
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of 
> Tomoya MORINAGA
> Sent: Wednesday, January 12, 2011 2:27 PM
> To: 'Milton Miller'; 'Grant Likely'
> Cc: 'David Brownell'; 
> spi-devel-general@lists.sourceforge.net; 
> linux-kernel@vger.kernel.org; yong.y.wang@intel.com; 
> joel.clark@intel.com; kok.howg.ewe@intel.com; 'Toshiharu 
> Okada'; 'Wang, Qi'
> Subject: RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213
> 
> On Tuesday, January 11, 2011 6:42 PM, Tomoya MORINAGA wrote:
> > However, I couldn't clear my question.
> 
> I show the current our coding image.
> Is the following agreeable with your saying ?
> 
> struct spi_platform_data {
> 	u16 num_chipselect;
> 	bool little_endian;
> 	u8 bits_per_word;
> 	struct spi_board_info *devices;
> 	u8 num_devices;
> };
> 
> static struct resource pch_spi_resources[] = {
> 	{
> 		.start 	= 0,
> 		.end	= 0,
> 		.flags	= IORESOURCE_MEM,
> 	},
> 	{
> 		.start	= 0,
> 		.end	= 0,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> };
> 
> static struct spi_platform_data pch_spi_platform_data = {
> 	.num_chipselect = 2,
> 	.little_endian = true,
> };
> 
> static struct pch_spi_cell pch_spi_cfg0 = {
> 	.name = "pch_spi",
> 	.num_resources = ARRAY_SIZE(pch_spi_resources),
> 	.resources = pch_spi_resources,
> 	.platform_data = &pch_spi_platform_data,
> 	.data_size = sizeof(pch_spi_platform_data), };
> 
> static int pch_spi_get_resources(struct pch_spi_board_data 
> *board_dat, int num) {
> 	int retval;
> 	int i;
> 
> 	/* create workqueue */
> 	board_dat->data[num]->wk = 
> create_singlethread_workqueue(KBUILD_MODNAME);
> 
> 	pci_request_regions(board_dat->pdev, KBUILD_MODNAME); }
> 
> static int pch_spi_add_device(struct device *parent, int id,
> 			  struct pch_spi_cell *cell,
> 			  struct resource *mem_base, int irq_base,
> 			      struct pch_spi_board_data *board_dat) {
> 
> 	platform_device_alloc(cell->name, id + cell->id);
> 
> 	res = kzalloc(sizeof(*res) * cell->num_resources, GFP_KERNEL);
> 	if (!res)
> 		goto fail_device;
> 
> 	platform_set_drvdata(pdev, cell->driver_data);
> 
> 	if (cell->data_size) {
> 		platform_device_add_data(pdev,
> 					cell->platform_data, 
> cell->data_size);
> 	}
> 
> 	for (i = 0; i < cell->num_resources; i++) {
> 		if ((cell->resources[i].flags & IORESOURCE_MEM) 
> && mem_base) {
> 			res[i].parent = 
> 			res[i].start = 
> 			res[i].end = 
> 		} else if (cell->resources[i].flags & IORESOURCE_IRQ) {
> 			res[i].start = 
> 			res[i].end   = 
> 		} else {
> 
> 
> 	}
> 
> 	ret = platform_device_add_resources(pdev, res, 
> cell->num_resources);
> 	if (ret)
> 		goto fail_res;
> 
> 	ret = platform_device_add(pdev);
> 	if (ret)
> 		goto fail_res;
> 
> 	kfree(res);
> 
> 	spi_alloc_master( );
> 
> 	board_dat->data[num] = spi_master_get_devdata( );
> 
> 	pch_spi_get_resources( );
> 
> 	/* set master mode */
> 	pch_spi_set_master_mode( );
> 
> 	/* Register the controller with the SPI core. */
> 	spi_register_master( );
> 
> 	return 0;
> }
> 
> int pch_spi_add_devices(struct device *parent,
> 		    struct pch_spi_cell *cells, int n_devs,
> 		    struct resource *mem_base,
> 		    struct pch_spi_board_data *board_dat) {
> 	pci_iomap()
> 
> 	for (i < n_devs)
> 		pch_spi_add_device()
> 
> }
> 
> static int pch_spi_probe(struct pci_dev *pdev, const struct 
> pci_device_id *id) {
> 	board_dat = kzalloc(sizeof(struct pch_spi_board_data), 
> GFP_KERNEL);
> 	pci_enable_device()
> 	request_irq()
> 	pch_spi_add_devices(&pdev->dev, 
> 			&pch_spi_cfg0,
> 			2,
> 			&pdev->resource[0], board_dat);
> 
> }
> 
> static irqreturn_t pch_spi_handler(int irq, void *dev_id) {
> 
> 	for (i = 0; i < board_dat->num; i++) {
> 		data = board_dat->data[i];
> 		io_remap_addr = data->io_remap_addr;
> 		spsr = io_remap_addr + PCH_SPSR;
> 
> 		reg_spsr_val = ioread32(spsr);
> 
> 		/* Check if the interrupt is for SPI device */
> 		if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
> 			pch_spi_handler_sub(data, reg_spsr_val, 
> io_remap_addr);
> 			ret = IRQ_HANDLED;
> 		}
> 
> 		dev_dbg(&board_dat->pdev->dev, "%s EXIT return 
> value=%d\n",
> 			__func__, ret);
> 	}
> 	return ret;
> }
> 
> Thanks,
> -----------------------------------------
> Tomoya MORINAGA
> OKI SEMICONDUCTOR CO., LTD.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in the body of a message to 
> majordomo@vger.kernel.org More majordomo info at  
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Grant Likely]

On Tue, Jan 18, 2011 at 09:36:48PM +0900, Tomoya MORINAGA wrote:
> Hi Grant,
> 
> Need your help!
> Could you give us your answer ?

Hi Tomoya, I've not forgotten about you, just a little swamped with
other stuff.  I'll try to get you a proper reply tomorrow.

g.

> 
> Thanks,
> -----------------------------------------
> Tomoya MORINAGA
> OKI SEMICONDUCTOR CO., LTD.
> 
> 
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org 
> > [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of 
> > Tomoya MORINAGA
> > Sent: Wednesday, January 12, 2011 2:27 PM
> > To: 'Milton Miller'; 'Grant Likely'
> > Cc: 'David Brownell'; 
> > spi-devel-general@lists.sourceforge.net; 
> > linux-kernel@vger.kernel.org; yong.y.wang@intel.com; 
> > joel.clark@intel.com; kok.howg.ewe@intel.com; 'Toshiharu 
> > Okada'; 'Wang, Qi'
> > Subject: RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213
> > 
> > On Tuesday, January 11, 2011 6:42 PM, Tomoya MORINAGA wrote:
> > > However, I couldn't clear my question.
> > 
> > I show the current our coding image.
> > Is the following agreeable with your saying ?
> > 
> > struct spi_platform_data {
> > 	u16 num_chipselect;
> > 	bool little_endian;
> > 	u8 bits_per_word;
> > 	struct spi_board_info *devices;
> > 	u8 num_devices;
> > };
> > 
> > static struct resource pch_spi_resources[] = {
> > 	{
> > 		.start 	= 0,
> > 		.end	= 0,
> > 		.flags	= IORESOURCE_MEM,
> > 	},
> > 	{
> > 		.start	= 0,
> > 		.end	= 0,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > };
> > 
> > static struct spi_platform_data pch_spi_platform_data = {
> > 	.num_chipselect = 2,
> > 	.little_endian = true,
> > };
> > 
> > static struct pch_spi_cell pch_spi_cfg0 = {
> > 	.name = "pch_spi",
> > 	.num_resources = ARRAY_SIZE(pch_spi_resources),
> > 	.resources = pch_spi_resources,
> > 	.platform_data = &pch_spi_platform_data,
> > 	.data_size = sizeof(pch_spi_platform_data), };
> > 
> > static int pch_spi_get_resources(struct pch_spi_board_data 
> > *board_dat, int num) {
> > 	int retval;
> > 	int i;
> > 
> > 	/* create workqueue */
> > 	board_dat->data[num]->wk = 
> > create_singlethread_workqueue(KBUILD_MODNAME);
> > 
> > 	pci_request_regions(board_dat->pdev, KBUILD_MODNAME); }
> > 
> > static int pch_spi_add_device(struct device *parent, int id,
> > 			  struct pch_spi_cell *cell,
> > 			  struct resource *mem_base, int irq_base,
> > 			      struct pch_spi_board_data *board_dat) {
> > 
> > 	platform_device_alloc(cell->name, id + cell->id);
> > 
> > 	res = kzalloc(sizeof(*res) * cell->num_resources, GFP_KERNEL);
> > 	if (!res)
> > 		goto fail_device;
> > 
> > 	platform_set_drvdata(pdev, cell->driver_data);
> > 
> > 	if (cell->data_size) {
> > 		platform_device_add_data(pdev,
> > 					cell->platform_data, 
> > cell->data_size);
> > 	}
> > 
> > 	for (i = 0; i < cell->num_resources; i++) {
> > 		if ((cell->resources[i].flags & IORESOURCE_MEM) 
> > && mem_base) {
> > 			res[i].parent = 
> > 			res[i].start = 
> > 			res[i].end = 
> > 		} else if (cell->resources[i].flags & IORESOURCE_IRQ) {
> > 			res[i].start = 
> > 			res[i].end   = 
> > 		} else {
> > 
> > 
> > 	}
> > 
> > 	ret = platform_device_add_resources(pdev, res, 
> > cell->num_resources);
> > 	if (ret)
> > 		goto fail_res;
> > 
> > 	ret = platform_device_add(pdev);
> > 	if (ret)
> > 		goto fail_res;
> > 
> > 	kfree(res);
> > 
> > 	spi_alloc_master( );
> > 
> > 	board_dat->data[num] = spi_master_get_devdata( );
> > 
> > 	pch_spi_get_resources( );
> > 
> > 	/* set master mode */
> > 	pch_spi_set_master_mode( );
> > 
> > 	/* Register the controller with the SPI core. */
> > 	spi_register_master( );
> > 
> > 	return 0;
> > }
> > 
> > int pch_spi_add_devices(struct device *parent,
> > 		    struct pch_spi_cell *cells, int n_devs,
> > 		    struct resource *mem_base,
> > 		    struct pch_spi_board_data *board_dat) {
> > 	pci_iomap()
> > 
> > 	for (i < n_devs)
> > 		pch_spi_add_device()
> > 
> > }
> > 
> > static int pch_spi_probe(struct pci_dev *pdev, const struct 
> > pci_device_id *id) {
> > 	board_dat = kzalloc(sizeof(struct pch_spi_board_data), 
> > GFP_KERNEL);
> > 	pci_enable_device()
> > 	request_irq()
> > 	pch_spi_add_devices(&pdev->dev, 
> > 			&pch_spi_cfg0,
> > 			2,
> > 			&pdev->resource[0], board_dat);
> > 
> > }
> > 
> > static irqreturn_t pch_spi_handler(int irq, void *dev_id) {
> > 
> > 	for (i = 0; i < board_dat->num; i++) {
> > 		data = board_dat->data[i];
> > 		io_remap_addr = data->io_remap_addr;
> > 		spsr = io_remap_addr + PCH_SPSR;
> > 
> > 		reg_spsr_val = ioread32(spsr);
> > 
> > 		/* Check if the interrupt is for SPI device */
> > 		if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) {
> > 			pch_spi_handler_sub(data, reg_spsr_val, 
> > io_remap_addr);
> > 			ret = IRQ_HANDLED;
> > 		}
> > 
> > 		dev_dbg(&board_dat->pdev->dev, "%s EXIT return 
> > value=%d\n",
> > 			__func__, ret);
> > 	}
> > 	return ret;
> > }
> > 
> > Thanks,
> > -----------------------------------------
> > Tomoya MORINAGA
> > OKI SEMICONDUCTOR CO., LTD.
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe 
> > linux-kernel" in the body of a message to 
> > majordomo@vger.kernel.org More majordomo info at  
> > http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 
> 

Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Grant Likely]

On Tue, Jan 11, 2011 at 06:38:35PM +0900, Tomoya MORINAGA wrote:
> Hi Grant,
> 
> On Friday, January 07, 2011 6:22 PM, Milton Miller wrote:
> > follow mfd_add_devices into drivers/mfd/mfd-core.c
> I have also seen mfd-core.c.
> However, I couldn't clear my question.
> 
> Q1. It seems platform_device is not used in your example. Does need "platform_device" ?
> Q2. How detect pch_spi channel dynamically ?
> 
> I want to catch your saying correctly.
> Could you give me more information ?

Let me try a different explanation.  The pch_spi driver implements a
pci_driver, and currently that pci_driver implements a single
spi_master child device as a child of the pci_bus.  The structure in
/sys/devices is probably something like this:

/sys/devices/.../0000:00:1a.0 (pci_dev bound to driver 'pch_spi_pcidev')
  |
  +--spi0.0  (spi_device 0 on spi_master 0)
  |
  \--spi0.1  (spi_device 1 on spi_master 0)

You want to modify the driver so that it can have more than one
spi_master attached to the device.  Your patch attempts to do so by
maintaining an array of spi_master devices that are children of the
pci_dev, so that the structure looks like this:


/sys/devices/.../0000:00:1a.0 (pci_dev bound to driver 'pch_spi_pcidev')
  |
  +--spi0.0  (spi_device 0 on spi_master 0)
  |
  +--spi0.1  (spi_device 1 on spi_master 0)
  |
  \--spi1.0  (spi_device 0 on spi_master 1)

Am I correct so far?

What I'm suggesting is that the code used to implement this structure
adds unnecessary complexity to the driver since each of the spi_master
instances are essentially completely independent, yet the driver has
to do extra work to know about the multiple instances.

Instead what you can do is to treat each spi bus as a child
platform_device of the pci_dev, and move the guts of the current
pch_spi_pcidev driver into a new platform_driver.  Then, instead of
driving the spi_master directly, the pch_spi_pcidev registers one or
more child platform_devices, one for each spi bus, so that the device
model looks something like this:

/sys/devices/.../0000:00:1a.0 (pci_dev bound to driver 'pch_spi_pcidev')
  |
  +--pch_spi.0
  |   |
  |   +--spi0.0  (spi_device 0 on spi_master 0)
  |   |
  |   \--spi0.1  (spi_device 1 on spi_master 0)
  |
  \--pch_spi.1
      |
      \--spi1.0  (spi_device 0 on spi_master 1)

where pch_spi.0 and pch_spi.1 are platform devices.  The
pch_spi_pcidev driver would allocated and register one or more
pch_spi.* platform_device children, each of which gets bound to a
pch_spi_platform_driver independently.  Doing it this way lets the
Linux driver model manage multiple instances instead of implementing
it by hand in your driver.

Make sense?

To do this, you need to move most of the driver code into a
platform_driver, and then modify your pci_driver so that it only
allocates and registers one or more platform_devices that will bind to
the driver.  It means you now have *two* driver structures.  One for
the pci_dev, and one for the platform_device children, and the pci_dev
acts as a container for the platform_devices.

The files in drivers/misc are an example of drivers that work in this
way.

g.


------------------------------------------------------------------------------
Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)!
Finally, a world-class log management solution at an even better price-free!
Download using promo code Free_Logger_4_Dev2Dev. Offer expires 
February 28th, so secure your free ArcSight Logger TODAY! 
http://p.sf.net/sfu/arcsight-sfd2d

RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213

[Tomoya MORINAGA]

Hi Grant,

Thank you for your explanation.

On Saturday, January 22, 2011 3:49 AM, Grant Likely wrote:
> Am I correct so far?

No, not true.
Both Topcliff(Intel EG20 PCH) and OKISEMI ML7213 IOH SPI devices
can't use multi-ChipSelect but 1 ChipSelect only.

I show the my modified diagram below.

- Currently upstreamed version,
 /sys/devices/.../0000:00:1a.0 (pci_dev bound to driver 'pch_spi_pcidev')
   |
   \--spi0.0  (spi_device 0 on spi_master 0)

- I posted before version.
 /sys/devices/.../0000:00:1a.0 (pci_dev bound to driver 'pch_spi_pcidev')
   |
   +--spi0.0  (spi_device 0 on spi_master 0)
   |
   \--spi1.0  (spi_device 0 on spi_master 1)

- Your anticipated becomes like below
 /sys/devices/.../0000:00:1a.0 (pci_dev bound to driver 'pch_spi_pcidev')
   |
   +--pch_spi.0
   |   |
   |   +--spi0.0  (spi_device 0 on spi_master 0)
   |
   \--pch_spi.1
       |
       \--spi1.0  (spi_device 0 on spi_master 1)


Still above,  don't you change your opinion?

> Make sense?
I probably understand your saying.
You mean insert platform_device as wrapper between pci_driver and spi_topcliff_pch.

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.


------------------------------------------------------------------------------
Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)!
Finally, a world-class log management solution at an even better price-free!
Download using promo code Free_Logger_4_Dev2Dev. Offer expires 
February 28th, so secure your free ArcSight Logger TODAY! 
http://p.sf.net/sfu/arcsight-sfd2d