* [PATCH 1/3] spi_topcliff_pch: support new device ML7213 @ 2010-12-27 11:23 Tomoya MORINAGA 2010-12-27 11:23 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Tomoya MORINAGA 2010-12-29 6:49 ` [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Grant Likely 0 siblings, 2 replies; 21+ messages in thread From: Tomoya MORINAGA @ 2010-12-27 11:23 UTC (permalink / raw) To: David Brownell, Grant Likely, spi-devel-general, linux-kernel Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove 2010-12-27 11:23 [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Tomoya MORINAGA @ 2010-12-27 11:23 ` Tomoya MORINAGA 2010-12-27 11:23 ` [PATCH 3/3] spi_topcliff_pch: fix resource leak issue Tomoya MORINAGA 2010-12-29 6:51 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Grant Likely 2010-12-29 6:49 ` [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Grant Likely 1 sibling, 2 replies; 21+ messages in thread From: Tomoya MORINAGA @ 2010-12-27 11:23 UTC (permalink / raw) To: David Brownell, Grant Likely, spi-devel-general, linux-kernel Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/3] spi_topcliff_pch: fix resource leak issue 2010-12-27 11:23 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Tomoya MORINAGA @ 2010-12-27 11:23 ` Tomoya MORINAGA 2010-12-29 6:51 ` Grant Likely 2010-12-29 6:51 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Grant Likely 1 sibling, 1 reply; 21+ messages in thread From: Tomoya MORINAGA @ 2010-12-27 11:23 UTC (permalink / raw) To: David Brownell, Grant Likely, spi-devel-general, linux-kernel Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] spi_topcliff_pch: fix resource leak issue 2010-12-27 11:23 ` [PATCH 3/3] spi_topcliff_pch: fix resource leak issue Tomoya MORINAGA @ 2010-12-29 6:51 ` Grant Likely 0 siblings, 0 replies; 21+ messages in thread From: Grant Likely @ 2010-12-29 6:51 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove 2010-12-27 11:23 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Tomoya MORINAGA 2010-12-27 11:23 ` [PATCH 3/3] spi_topcliff_pch: fix resource leak issue Tomoya MORINAGA @ 2010-12-29 6:51 ` Grant Likely 1 sibling, 0 replies; 21+ messages in thread From: Grant Likely @ 2010-12-29 6:51 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2010-12-27 11:23 [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Tomoya MORINAGA 2010-12-27 11:23 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Tomoya MORINAGA @ 2010-12-29 6:49 ` Grant Likely 2011-01-05 2:34 ` Tomoya MORINAGA 1 sibling, 1 reply; 21+ messages in thread From: Grant Likely @ 2010-12-29 6:49 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2010-12-29 6:49 ` [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Grant Likely @ 2011-01-05 2:34 ` Tomoya MORINAGA 2011-01-05 16:38 ` Grant Likely 0 siblings, 1 reply; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-05 2:34 UTC (permalink / raw) To: Grant Likely Cc: David Brownell, spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Toshiharu Okada 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-05 2:34 ` Tomoya MORINAGA @ 2011-01-05 16:38 ` Grant Likely 2011-01-07 6:40 ` Tomoya MORINAGA 2011-01-07 6:47 ` Tomoya MORINAGA 0 siblings, 2 replies; 21+ messages in thread From: Grant Likely @ 2011-01-05 16:38 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Toshiharu Okada 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. > ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-05 16:38 ` Grant Likely @ 2011-01-07 6:40 ` Tomoya MORINAGA [not found] ` <46862922F48B45F781832619CF3F4B79-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> 2011-01-07 9:57 ` Milton Miller 2011-01-07 6:47 ` Tomoya MORINAGA 1 sibling, 2 replies; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-07 6:40 UTC (permalink / raw) To: 'Grant Likely', 'Tomoya MORINAGA' Cc: 'David Brownell', spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, 'Toshiharu Okada' 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. > > > ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <46862922F48B45F781832619CF3F4B79-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 [not found] ` <46862922F48B45F781832619CF3F4B79-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> @ 2011-01-07 9:21 ` Milton Miller [not found] ` <topcliff-split-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org> 2011-01-11 9:42 ` Tomoya MORINAGA 2011-01-07 9:45 ` Milton Miller 1 sibling, 2 replies; 21+ messages in thread From: Milton Miller @ 2011-01-07 9:21 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, qi.wang-ral2JQCrhuEAvxtiuMwx3w, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, joel.clark-ral2JQCrhuEAvxtiuMwx3w 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <topcliff-split-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org>]
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 [not found] ` <topcliff-split-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org> @ 2011-01-07 9:45 ` Tomoya MORINAGA 2011-01-11 9:38 ` Tomoya MORINAGA 1 sibling, 0 replies; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-07 9:45 UTC (permalink / raw) To: 'Milton Miller' Cc: 'David Brownell', yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, 'linux-kernel-u79uwXL29TaiAVqoAR/hOA49XUYAGyU/ 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 [not found] ` <topcliff-split-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org> 2011-01-07 9:45 ` Tomoya MORINAGA @ 2011-01-11 9:38 ` Tomoya MORINAGA [not found] ` <84FF3503839A46E0B9919DF000DDE5C5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> 1 sibling, 1 reply; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-11 9:38 UTC (permalink / raw) To: 'Milton Miller', 'Grant Likely' Cc: 'David Brownell', 'Wang, Qi', yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, Toshiharu Okada, kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, 'linux-kernel-u79uwXL29TaiAVqoAR/hOA49XUYAGyU/ 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <84FF3503839A46E0B9919DF000DDE5C5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 [not found] ` <84FF3503839A46E0B9919DF000DDE5C5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> @ 2011-01-21 18:48 ` Grant Likely [not found] ` <20110121184852.GA18603-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Grant Likely @ 2011-01-21 18:48 UTC (permalink / raw) To: Tomoya MORINAGA Cc: 'David Brownell', 'Wang, Qi', 'linux-kernel-u79uwXL29TaiAVqoAR/hOA49XUYAGyU/@public.gmane.org@intel.com', yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, 'Milton Miller', Toshiharu Okada, kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, joel.clark-ral2JQCrhuEAvxtiuMwx3w 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20110121184852.GA18603-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>]
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 [not found] ` <20110121184852.GA18603-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org> @ 2011-01-24 3:14 ` Tomoya MORINAGA 0 siblings, 0 replies; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-24 3:14 UTC (permalink / raw) To: 'Grant Likely' Cc: 'David Brownell', 'Wang, Qi', 'linux-kernel-u79uwXL29TaiAVqoAR/hOA49XUYAGyU/@public.gmane.org@intel.com', yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, 'Milton Miller', 'Toshiharu Okada', kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, joel.clark-ral2JQCrhuEAvxtiuMwx3w 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-07 9:21 ` Milton Miller [not found] ` <topcliff-split-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org> @ 2011-01-11 9:42 ` Tomoya MORINAGA 2011-01-12 5:27 ` Tomoya MORINAGA 1 sibling, 1 reply; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-11 9:42 UTC (permalink / raw) To: 'Milton Miller', 'Grant Likely' Cc: 'David Brownell', spi-devel-general, linux-kernel, yong.y.wang, joel.clark, kok.howg.ewe, Toshiharu Okada, 'Wang, Qi' 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-11 9:42 ` Tomoya MORINAGA @ 2011-01-12 5:27 ` Tomoya MORINAGA 2011-01-18 12:36 ` Tomoya MORINAGA 0 siblings, 1 reply; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-12 5:27 UTC (permalink / raw) To: 'Milton Miller', 'Grant Likely' Cc: 'David Brownell', spi-devel-general, linux-kernel, yong.y.wang, joel.clark, kok.howg.ewe, 'Toshiharu Okada', 'Wang, Qi' 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-12 5:27 ` Tomoya MORINAGA @ 2011-01-18 12:36 ` Tomoya MORINAGA 2011-01-19 6:25 ` Grant Likely 0 siblings, 1 reply; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-18 12:36 UTC (permalink / raw) To: 'Milton Miller', 'Grant Likely' Cc: 'David Brownell', spi-devel-general, linux-kernel, yong.y.wang, joel.clark, kok.howg.ewe, 'Toshiharu Okada', 'Wang, Qi' 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/ > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-18 12:36 ` Tomoya MORINAGA @ 2011-01-19 6:25 ` Grant Likely 0 siblings, 0 replies; 21+ messages in thread From: Grant Likely @ 2011-01-19 6:25 UTC (permalink / raw) To: Tomoya MORINAGA Cc: 'Milton Miller', 'David Brownell', spi-devel-general, linux-kernel, yong.y.wang, joel.clark, kok.howg.ewe, 'Toshiharu Okada', 'Wang, Qi' 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/ > > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 [not found] ` <46862922F48B45F781832619CF3F4B79-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> 2011-01-07 9:21 ` Milton Miller @ 2011-01-07 9:45 ` Milton Miller 1 sibling, 0 replies; 21+ messages in thread From: Milton Miller @ 2011-01-07 9:45 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, qi.wang-ral2JQCrhuEAvxtiuMwx3w, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, joel.clark-ral2JQCrhuEAvxtiuMwx3w 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-07 6:40 ` Tomoya MORINAGA [not found] ` <46862922F48B45F781832619CF3F4B79-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> @ 2011-01-07 9:57 ` Milton Miller 1 sibling, 0 replies; 21+ messages in thread From: Milton Miller @ 2011-01-07 9:57 UTC (permalink / raw) To: Tomoya MORINAGA Cc: David Brownell, Grant Likely, spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] spi_topcliff_pch: support new device ML7213 2011-01-05 16:38 ` Grant Likely 2011-01-07 6:40 ` Tomoya MORINAGA @ 2011-01-07 6:47 ` Tomoya MORINAGA 1 sibling, 0 replies; 21+ messages in thread From: Tomoya MORINAGA @ 2011-01-07 6:47 UTC (permalink / raw) To: 'Grant Likely' Cc: 'David Brownell', spi-devel-general, linux-kernel, qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, 'Toshiharu Okada' 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-01-24 3:14 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-12-27 11:23 [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Tomoya MORINAGA 2010-12-27 11:23 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Tomoya MORINAGA 2010-12-27 11:23 ` [PATCH 3/3] spi_topcliff_pch: fix resource leak issue Tomoya MORINAGA 2010-12-29 6:51 ` Grant Likely 2010-12-29 6:51 ` [PATCH 2/3] spi_topcliff_pch: change calling function order correctly in remove Grant Likely 2010-12-29 6:49 ` [PATCH 1/3] spi_topcliff_pch: support new device ML7213 Grant Likely 2011-01-05 2:34 ` Tomoya MORINAGA 2011-01-05 16:38 ` Grant Likely 2011-01-07 6:40 ` Tomoya MORINAGA [not found] ` <46862922F48B45F781832619CF3F4B79-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> 2011-01-07 9:21 ` Milton Miller [not found] ` <topcliff-split-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org> 2011-01-07 9:45 ` Tomoya MORINAGA 2011-01-11 9:38 ` Tomoya MORINAGA [not found] ` <84FF3503839A46E0B9919DF000DDE5C5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org> 2011-01-21 18:48 ` Grant Likely [not found] ` <20110121184852.GA18603-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org> 2011-01-24 3:14 ` Tomoya MORINAGA 2011-01-11 9:42 ` Tomoya MORINAGA 2011-01-12 5:27 ` Tomoya MORINAGA 2011-01-18 12:36 ` Tomoya MORINAGA 2011-01-19 6:25 ` Grant Likely 2011-01-07 9:45 ` Milton Miller 2011-01-07 9:57 ` Milton Miller 2011-01-07 6:47 ` Tomoya MORINAGA
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).