* [PATCH 0/8 RESEND] sta2x11-mfd patches
@ 2012-09-12 10:22 ciminaghi
2012-09-12 10:22 ` [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code ciminaghi
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw)
To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi
From: Davide Ciminaghi <ciminaghi@gnudd.com>
Hi,
this is a resend of a patchset dated Jun 10th (see
https://lkml.org/lkml/2012/6/10/188) even if some of the
commits have been squashed.
I'm working on the implementation of the common clock framework
for the STA2X11 (aka CONNEXT) soc.
To configure, enable and disable clocks, some general purpose
registers (apb-soc-registers) must be accessed. Such registers
can be reached through bar #1 of a multifunction pci device,
which is not covered by the current version of sta2x11-mfd.
This patch series adds support for the apb-soc-registers device
and rearranges the sta2x11-mfd driver to avoid unnecessary
code duplications and to cope with the increased number of platform
devices.
Davide
Davide Ciminaghi (8):
sta2x11-mfd : add apb-soc regs driver and factor out common code
sta2x11-mfd : add sta2x11_mfd_get_regs_data() function
sta2x11-mfd : use defines for platform devices' names
sta2x11-mfd : only add sta2x11_mfd if it hasn't already been added
sta2x11-mfd : platform probe: don't mind about gpio platform data
sta2x11-mfd : use one lock per device instead of one lock per mfd.
sta2x11-mfd : add defines for some sta2x11 sctl registers.
sta2x11-mfd : add myself to copyright
drivers/mfd/sta2x11-mfd.c | 416 ++++++++++++++++++++++++++-------------
include/linux/mfd/sta2x11-mfd.h | 190 ++++++++++++++++++-
2 files changed, 470 insertions(+), 136 deletions(-)
--
1.7.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-25 19:20 ` Mark Brown 2012-09-12 10:22 ` [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function ciminaghi ` (7 subsequent siblings) 8 siblings, 1 reply; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> A driver for the apb-soc registers is needed by the clock infrastructure code to configure and control clocks on the sta2x11 chip. Since some of the functions in sta2x11-mfd.c were almost identical for the two existing platform devices, the following changes have been performed to avoid further code duplication while adding the apb-soc-regs driver: * The sctl_regs and apbreg_regs fields in struct sta2x11_mfd have been turned into just one array of pointers accessed by device index. * Platform probe methods have become one-liners invoking a common probe with the device's index as second parameter. * For loops have been inserted where the same operations were performed for each of the two bars of a pci device. * The apbreg_mask and sctl_mask functions were almost identical, so they were turned into inline functions invoking a common __sta2x11_mfd_mask() with the platform device's index as last parameter. To do this, enum sta2x11_mfd_plat_dev has been declared in sta2x11-mfd.h and more device types have been added to it. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 350 +++++++++++++++++++++++++++------------ include/linux/mfd/sta2x11-mfd.h | 156 +++++++++++++++++- 2 files changed, 395 insertions(+), 111 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index d31fed0..5fbd044 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -40,8 +40,7 @@ struct sta2x11_mfd { struct sta2x11_instance *instance; spinlock_t lock; struct list_head list; - void __iomem *sctl_regs; - void __iomem *apbreg_regs; + void __iomem *regs[sta2x11_n_mfd_plat_devs]; }; static LIST_HEAD(sta2x11_mfd_list); @@ -100,56 +99,33 @@ static int __devexit mfd_remove(struct pci_dev *pdev) return 0; } -/* These two functions are exported and are not expected to fail */ -u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) +/* This function is exported and is not expected to fail */ +u32 __sta2x11_mfd_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val, + enum sta2x11_mfd_plat_dev index) { struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev); u32 r; unsigned long flags; + void __iomem *regs = mfd->regs[index]; if (!mfd) { dev_warn(&pdev->dev, ": can't access sctl regs\n"); return 0; } - if (!mfd->sctl_regs) { + if (!regs) { dev_warn(&pdev->dev, ": system ctl not initialized\n"); return 0; } spin_lock_irqsave(&mfd->lock, flags); - r = readl(mfd->sctl_regs + reg); + r = readl(regs + reg); r &= ~mask; r |= val; if (mask) - writel(r, mfd->sctl_regs + reg); + writel(r, regs + reg); spin_unlock_irqrestore(&mfd->lock, flags); return r; } -EXPORT_SYMBOL(sta2x11_sctl_mask); - -u32 sta2x11_apbreg_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) -{ - struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev); - u32 r; - unsigned long flags; - - if (!mfd) { - dev_warn(&pdev->dev, ": can't access apb regs\n"); - return 0; - } - if (!mfd->apbreg_regs) { - dev_warn(&pdev->dev, ": apb bridge not initialized\n"); - return 0; - } - spin_lock_irqsave(&mfd->lock, flags); - r = readl(mfd->apbreg_regs + reg); - r &= ~mask; - r |= val; - if (mask) - writel(r, mfd->apbreg_regs + reg); - spin_unlock_irqrestore(&mfd->lock, flags); - return r; -} -EXPORT_SYMBOL(sta2x11_apbreg_mask); +EXPORT_SYMBOL(__sta2x11_mfd_mask); /* Two debugfs files, for our registers (FIXME: one instance only) */ #define REG(regname) {.name = #regname, .offset = SCTL_ ## regname} @@ -180,51 +156,103 @@ static struct debugfs_regset32 apbreg_regset = { .nregs = ARRAY_SIZE(sta2x11_apbreg_regs), }; -static struct dentry *sta2x11_sctl_debugfs; -static struct dentry *sta2x11_apbreg_debugfs; +#define REG(regname) {.name = #regname, .offset = regname} +static struct debugfs_reg32 sta2x11_apb_soc_regs_regs[] = { + REG(PCIE_EP1_FUNC3_0_INTR_REG), REG(PCIE_EP1_FUNC7_4_INTR_REG), + REG(PCIE_EP2_FUNC3_0_INTR_REG), REG(PCIE_EP2_FUNC7_4_INTR_REG), + REG(PCIE_EP3_FUNC3_0_INTR_REG), REG(PCIE_EP3_FUNC7_4_INTR_REG), + REG(PCIE_EP4_FUNC3_0_INTR_REG), REG(PCIE_EP4_FUNC7_4_INTR_REG), + REG(PCIE_INTR_ENABLE0_REG), REG(PCIE_INTR_ENABLE1_REG), + REG(PCIE_EP1_FUNC_TC_REG), REG(PCIE_EP2_FUNC_TC_REG), + REG(PCIE_EP3_FUNC_TC_REG), REG(PCIE_EP4_FUNC_TC_REG), + REG(PCIE_EP1_FUNC_F_REG), REG(PCIE_EP2_FUNC_F_REG), + REG(PCIE_EP3_FUNC_F_REG), REG(PCIE_EP4_FUNC_F_REG), + REG(PCIE_PAB_AMBA_SW_RST_REG), REG(PCIE_PM_STATUS_0_PORT_0_4), + REG(PCIE_PM_STATUS_7_0_EP1), REG(PCIE_PM_STATUS_7_0_EP2), + REG(PCIE_PM_STATUS_7_0_EP3), REG(PCIE_PM_STATUS_7_0_EP4), + REG(PCIE_DEV_ID_0_EP1_REG), REG(PCIE_CC_REV_ID_0_EP1_REG), + REG(PCIE_DEV_ID_1_EP1_REG), REG(PCIE_CC_REV_ID_1_EP1_REG), + REG(PCIE_DEV_ID_2_EP1_REG), REG(PCIE_CC_REV_ID_2_EP1_REG), + REG(PCIE_DEV_ID_3_EP1_REG), REG(PCIE_CC_REV_ID_3_EP1_REG), + REG(PCIE_DEV_ID_4_EP1_REG), REG(PCIE_CC_REV_ID_4_EP1_REG), + REG(PCIE_DEV_ID_5_EP1_REG), REG(PCIE_CC_REV_ID_5_EP1_REG), + REG(PCIE_DEV_ID_6_EP1_REG), REG(PCIE_CC_REV_ID_6_EP1_REG), + REG(PCIE_DEV_ID_7_EP1_REG), REG(PCIE_CC_REV_ID_7_EP1_REG), + REG(PCIE_DEV_ID_0_EP2_REG), REG(PCIE_CC_REV_ID_0_EP2_REG), + REG(PCIE_DEV_ID_1_EP2_REG), REG(PCIE_CC_REV_ID_1_EP2_REG), + REG(PCIE_DEV_ID_2_EP2_REG), REG(PCIE_CC_REV_ID_2_EP2_REG), + REG(PCIE_DEV_ID_3_EP2_REG), REG(PCIE_CC_REV_ID_3_EP2_REG), + REG(PCIE_DEV_ID_4_EP2_REG), REG(PCIE_CC_REV_ID_4_EP2_REG), + REG(PCIE_DEV_ID_5_EP2_REG), REG(PCIE_CC_REV_ID_5_EP2_REG), + REG(PCIE_DEV_ID_6_EP2_REG), REG(PCIE_CC_REV_ID_6_EP2_REG), + REG(PCIE_DEV_ID_7_EP2_REG), REG(PCIE_CC_REV_ID_7_EP2_REG), + REG(PCIE_DEV_ID_0_EP3_REG), REG(PCIE_CC_REV_ID_0_EP3_REG), + REG(PCIE_DEV_ID_1_EP3_REG), REG(PCIE_CC_REV_ID_1_EP3_REG), + REG(PCIE_DEV_ID_2_EP3_REG), REG(PCIE_CC_REV_ID_2_EP3_REG), + REG(PCIE_DEV_ID_3_EP3_REG), REG(PCIE_CC_REV_ID_3_EP3_REG), + REG(PCIE_DEV_ID_4_EP3_REG), REG(PCIE_CC_REV_ID_4_EP3_REG), + REG(PCIE_DEV_ID_5_EP3_REG), REG(PCIE_CC_REV_ID_5_EP3_REG), + REG(PCIE_DEV_ID_6_EP3_REG), REG(PCIE_CC_REV_ID_6_EP3_REG), + REG(PCIE_DEV_ID_7_EP3_REG), REG(PCIE_CC_REV_ID_7_EP3_REG), + REG(PCIE_DEV_ID_0_EP4_REG), REG(PCIE_CC_REV_ID_0_EP4_REG), + REG(PCIE_DEV_ID_1_EP4_REG), REG(PCIE_CC_REV_ID_1_EP4_REG), + REG(PCIE_DEV_ID_2_EP4_REG), REG(PCIE_CC_REV_ID_2_EP4_REG), + REG(PCIE_DEV_ID_3_EP4_REG), REG(PCIE_CC_REV_ID_3_EP4_REG), + REG(PCIE_DEV_ID_4_EP4_REG), REG(PCIE_CC_REV_ID_4_EP4_REG), + REG(PCIE_DEV_ID_5_EP4_REG), REG(PCIE_CC_REV_ID_5_EP4_REG), + REG(PCIE_DEV_ID_6_EP4_REG), REG(PCIE_CC_REV_ID_6_EP4_REG), + REG(PCIE_DEV_ID_7_EP4_REG), REG(PCIE_CC_REV_ID_7_EP4_REG), + REG(PCIE_SUBSYS_VEN_ID_REG), REG(PCIE_COMMON_CLOCK_CONFIG_0_4_0), + REG(PCIE_MIPHYP_SSC_EN_REG), REG(PCIE_MIPHYP_ADDR_REG), + REG(PCIE_L1_ASPM_READY_REG), REG(PCIE_EXT_CFG_RDY_REG), + REG(PCIE_SoC_INT_ROUTER_STATUS0_REG), + REG(PCIE_SoC_INT_ROUTER_STATUS1_REG), + REG(PCIE_SoC_INT_ROUTER_STATUS2_REG), + REG(PCIE_SoC_INT_ROUTER_STATUS3_REG), + REG(DMA_IP_CTRL_REG), REG(DISP_BRIDGE_PU_PD_CTRL_REG), + REG(VIP_PU_PD_CTRL_REG), REG(USB_MLB_PU_PD_CTRL_REG), + REG(SDIO_PU_PD_MISCFUNC_CTRL_REG1), REG(SDIO_PU_PD_MISCFUNC_CTRL_REG2), + REG(UART_PU_PD_CTRL_REG), REG(ARM_Lock), REG(SYS_IO_CHAR_REG1), + REG(SYS_IO_CHAR_REG2), REG(SATA_CORE_ID_REG), REG(SATA_CTRL_REG), + REG(I2C_HSFIX_MISC_REG), REG(SPARE2_RESERVED), REG(SPARE3_RESERVED), + REG(MASTER_LOCK_REG), REG(SYSTEM_CONFIG_STATUS_REG), + REG(MSP_CLK_CTRL_REG), REG(COMPENSATION_REG1), REG(COMPENSATION_REG2), + REG(COMPENSATION_REG3), REG(TEST_CTL_REG), +}; +#undef REG -/* Probe for the two platform devices */ -static int sta2x11_sctl_probe(struct platform_device *dev) -{ - struct pci_dev **pdev; - struct sta2x11_mfd *mfd; - struct resource *res; +static struct debugfs_regset32 apb_soc_regs_regset = { + .regs = sta2x11_apb_soc_regs_regs, + .nregs = ARRAY_SIZE(sta2x11_apb_soc_regs_regs), +}; - pdev = dev->dev.platform_data; - mfd = sta2x11_mfd_find(*pdev); - if (!mfd) - return -ENODEV; - res = platform_get_resource(dev, IORESOURCE_MEM, 0); - if (!res) - return -ENOMEM; +static struct dentry *sta2x11_mfd_debugfs[sta2x11_n_mfd_plat_devs]; - if (!request_mem_region(res->start, resource_size(res), - "sta2x11-sctl")) - return -EBUSY; +static struct debugfs_regset32 *sta2x11_mfd_regset[sta2x11_n_mfd_plat_devs] = { + [sta2x11_sctl] = &sctl_regset, + [sta2x11_apbreg] = &apbreg_regset, + [sta2x11_apb_soc_regs] = &apb_soc_regs_regset, +}; - mfd->sctl_regs = ioremap(res->start, resource_size(res)); - if (!mfd->sctl_regs) { - release_mem_region(res->start, resource_size(res)); - return -ENOMEM; - } - sctl_regset.base = mfd->sctl_regs; - sta2x11_sctl_debugfs = debugfs_create_regset32("sta2x11-sctl", - S_IFREG | S_IRUGO, - NULL, &sctl_regset); - return 0; -} +static const char *sta2x11_mfd_names[sta2x11_n_mfd_plat_devs] = { + [sta2x11_sctl] = "sta2x11-sctl", + [sta2x11_apbreg] = "sta2x11-apbreg", + [sta2x11_apb_soc_regs] = "sta2x11-apb-soc-regs", +}; -static int sta2x11_apbreg_probe(struct platform_device *dev) +/* Probe for the three platform devices */ + +static int sta2x11_mfd_platform_probe(struct platform_device *dev, + enum sta2x11_mfd_plat_dev index) { struct pci_dev **pdev; struct sta2x11_mfd *mfd; struct resource *res; + const char *name = sta2x11_mfd_names[index]; + struct debugfs_regset32 *regset = sta2x11_mfd_regset[index]; pdev = dev->dev.platform_data; - dev_dbg(&dev->dev, "%s: pdata is %p\n", __func__, pdev); - dev_dbg(&dev->dev, "%s: *pdata is %p\n", __func__, *pdev); - mfd = sta2x11_mfd_find(*pdev); if (!mfd) return -ENODEV; @@ -233,25 +261,37 @@ static int sta2x11_apbreg_probe(struct platform_device *dev) if (!res) return -ENOMEM; - if (!request_mem_region(res->start, resource_size(res), - "sta2x11-apbreg")) + if (!request_mem_region(res->start, resource_size(res), name)) return -EBUSY; - mfd->apbreg_regs = ioremap(res->start, resource_size(res)); - if (!mfd->apbreg_regs) { + mfd->regs[index] = ioremap(res->start, resource_size(res)); + if (!mfd->regs[index]) { release_mem_region(res->start, resource_size(res)); return -ENOMEM; } - dev_dbg(&dev->dev, "%s: regbase %p\n", __func__, mfd->apbreg_regs); - - apbreg_regset.base = mfd->apbreg_regs; - sta2x11_apbreg_debugfs = debugfs_create_regset32("sta2x11-apbreg", - S_IFREG | S_IRUGO, - NULL, &apbreg_regset); + regset->base = mfd->regs[index]; + sta2x11_mfd_debugfs[index] = debugfs_create_regset32(name, + S_IFREG | S_IRUGO, + NULL, regset); return 0; } -/* The two platform drivers */ +static int sta2x11_sctl_probe(struct platform_device *dev) +{ + return sta2x11_mfd_platform_probe(dev, sta2x11_sctl); +} + +static int sta2x11_apbreg_probe(struct platform_device *dev) +{ + return sta2x11_mfd_platform_probe(dev, sta2x11_apbreg); +} + +static int sta2x11_apb_soc_regs_probe(struct platform_device *dev) +{ + return sta2x11_mfd_platform_probe(dev, sta2x11_apb_soc_regs); +} + +/* The three platform drivers */ static struct platform_driver sta2x11_sctl_platform_driver = { .driver = { .name = "sta2x11-sctl", @@ -280,13 +320,29 @@ static int __init sta2x11_apbreg_init(void) return platform_driver_register(&sta2x11_platform_driver); } +static struct platform_driver sta2x11_apb_soc_regs_platform_driver = { + .driver = { + .name = "sta2x11-apb-soc-regs", + .owner = THIS_MODULE, + }, + .probe = sta2x11_apb_soc_regs_probe, +}; + +static int __init sta2x11_apb_soc_regs_init(void) +{ + pr_info("%s\n", __func__); + return platform_driver_register(&sta2x11_apb_soc_regs_platform_driver); +} + /* - * What follows is the PCI device that hosts the above two pdevs. + * What follows are the PCI devices that host the above pdevs. * Each logic block is 4kB and they are all consecutive: we use this info. */ -/* Bar 0 */ -enum bar0_cells { +/* Mfd 0 device */ + +/* Mfd 0, Bar 0 */ +enum mfd0_bar0_cells { STA2X11_GPIO_0 = 0, STA2X11_GPIO_1, STA2X11_GPIO_2, @@ -295,8 +351,8 @@ enum bar0_cells { STA2X11_SCR, STA2X11_TIME, }; -/* Bar 1 */ -enum bar1_cells { +/* Mfd 0 , Bar 1 */ +enum mfd0_bar1_cells { STA2X11_APBREG = 0, }; #define CELL_4K(_name, _cell) { \ @@ -330,17 +386,46 @@ static const __devinitconst struct resource apbreg_resources[] = { #define DEV(_name, _r) \ { .name = _name, .num_resources = ARRAY_SIZE(_r), .resources = _r, } -static __devinitdata struct mfd_cell sta2x11_mfd_bar0[] = { +static __devinitdata struct mfd_cell sta2x11_mfd0_bar0[] = { DEV("sta2x11-gpio", gpio_resources), /* offset 0: we add pdata later */ DEV("sta2x11-sctl", sctl_resources), DEV("sta2x11-scr", scr_resources), DEV("sta2x11-time", time_resources), }; -static __devinitdata struct mfd_cell sta2x11_mfd_bar1[] = { +static __devinitdata struct mfd_cell sta2x11_mfd0_bar1[] = { DEV("sta2x11-apbreg", apbreg_resources), }; +/* Mfd 1 devices */ + +/* Mfd 1, Bar 0 */ +enum mfd1_bar0_cells { + STA2X11_VIC = 0, +}; + +/* Mfd 1, Bar 1 */ +enum mfd1_bar1_cells { + STA2X11_APB_SOC_REGS = 0, +}; + +static const __devinitconst struct resource vic_resources[] = { + CELL_4K("sta2x11-vic", STA2X11_VIC), +}; + +static const __devinitconst struct resource apb_soc_regs_resources[] = { + CELL_4K("sta2x11-apb-soc-regs", STA2X11_APB_SOC_REGS), +}; + +static __devinitdata struct mfd_cell sta2x11_mfd1_bar0[] = { + DEV("sta2x11-vic", vic_resources), +}; + +static __devinitdata struct mfd_cell sta2x11_mfd1_bar1[] = { + DEV("sta2x11-apb-soc-regs", apb_soc_regs_resources), +}; + + static int sta2x11_mfd_suspend(struct pci_dev *pdev, pm_message_t state) { pci_save_state(pdev); @@ -363,10 +448,63 @@ static int sta2x11_mfd_resume(struct pci_dev *pdev) return 0; } +struct sta2x11_mfd_bar_setup_data { + struct mfd_cell *cells; + int ncells; +}; + +struct sta2x11_mfd_setup_data { + struct sta2x11_mfd_bar_setup_data bars[2]; +}; + +#define STA2X11_MFD0 0 +#define STA2X11_MFD1 1 + +static struct sta2x11_mfd_setup_data mfd_setup_data[] = { + /* Mfd 0: gpio, sctl, scr, timers / apbregs */ + [STA2X11_MFD0] = { + .bars = { + [0] = { + .cells = sta2x11_mfd0_bar0, + .ncells = ARRAY_SIZE(sta2x11_mfd0_bar0), + }, + [1] = { + .cells = sta2x11_mfd0_bar1, + .ncells = ARRAY_SIZE(sta2x11_mfd0_bar1), + }, + }, + }, + /* Mfd 1: vic / apb-soc-regs */ + [STA2X11_MFD1] = { + .bars = { + [0] = { + .cells = sta2x11_mfd1_bar0, + .ncells = ARRAY_SIZE(sta2x11_mfd1_bar0), + }, + [1] = { + .cells = sta2x11_mfd1_bar1, + .ncells = ARRAY_SIZE(sta2x11_mfd1_bar1), + }, + }, + }, +}; + +static void __devinit sta2x11_mfd_setup(struct pci_dev *pdev, + struct sta2x11_mfd_setup_data *sd) +{ + int i, j; + for (i = 0; i < ARRAY_SIZE(sd->bars); i++) + for (j = 0; j < sd->bars[i].ncells; j++) { + sd->bars[i].cells[j].pdata_size = sizeof(pdev); + sd->bars[i].cells[j].platform_data = &pdev; + } +} + static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id) { int err, i; + struct sta2x11_mfd_setup_data *setup_data; struct sta2x11_gpio_pdata *gpio_data; dev_info(&pdev->dev, "%s\n", __func__); @@ -381,6 +519,10 @@ static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev, if (err) dev_info(&pdev->dev, "Enable msi failed\n"); + setup_data = pci_id->device == PCI_DEVICE_ID_STMICRO_GPIO ? + &mfd_setup_data[STA2X11_MFD0] : + &mfd_setup_data[STA2X11_MFD1]; + /* Read gpio config data as pci device's platform data */ gpio_data = dev_get_platdata(&pdev->dev); if (!gpio_data) @@ -392,35 +534,23 @@ static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev, pdev, &pdev); /* platform data is the pci device for all of them */ - for (i = 0; i < ARRAY_SIZE(sta2x11_mfd_bar0); i++) { - sta2x11_mfd_bar0[i].pdata_size = sizeof(pdev); - sta2x11_mfd_bar0[i].platform_data = &pdev; - } - sta2x11_mfd_bar1[0].pdata_size = sizeof(pdev); - sta2x11_mfd_bar1[0].platform_data = &pdev; + sta2x11_mfd_setup(pdev, setup_data); /* Record this pdev before mfd_add_devices: their probe looks for it */ sta2x11_mfd_add(pdev, GFP_ATOMIC); - - err = mfd_add_devices(&pdev->dev, -1, - sta2x11_mfd_bar0, - ARRAY_SIZE(sta2x11_mfd_bar0), - &pdev->resource[0], - 0); - if (err) { - dev_err(&pdev->dev, "mfd_add_devices[0] failed: %d\n", err); - goto err_disable; - } - - err = mfd_add_devices(&pdev->dev, -1, - sta2x11_mfd_bar1, - ARRAY_SIZE(sta2x11_mfd_bar1), - &pdev->resource[1], - 0); - if (err) { - dev_err(&pdev->dev, "mfd_add_devices[1] failed: %d\n", err); - goto err_disable; + /* Just 2 bars for all mfd's at present */ + for (i = 0; i < 2; i++) { + err = mfd_add_devices(&pdev->dev, -1, + setup_data->bars[i].cells, + setup_data->bars[i].ncells, + &pdev->resource[i], + 0); + if (err) { + dev_err(&pdev->dev, + "mfd_add_devices[%d] failed: %d\n", i, err); + goto err_disable; + } } return 0; @@ -434,6 +564,7 @@ err_disable: static DEFINE_PCI_DEVICE_TABLE(sta2x11_mfd_tbl) = { {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_GPIO)}, + {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_VIC)}, {0,}, }; @@ -459,6 +590,7 @@ static int __init sta2x11_mfd_init(void) */ subsys_initcall(sta2x11_apbreg_init); subsys_initcall(sta2x11_sctl_init); +subsys_initcall(sta2x11_apb_soc_regs_init); rootfs_initcall(sta2x11_mfd_init); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/mfd/sta2x11-mfd.h b/include/linux/mfd/sta2x11-mfd.h index d179227..4d85879 100644 --- a/include/linux/mfd/sta2x11-mfd.h +++ b/include/linux/mfd/sta2x11-mfd.h @@ -26,6 +26,20 @@ #include <linux/types.h> #include <linux/pci.h> +enum sta2x11_mfd_plat_dev { + sta2x11_sctl = 0, + sta2x11_gpio, + sta2x11_scr, + sta2x11_time, + sta2x11_apbreg, + sta2x11_apb_soc_regs, + sta2x11_vic, + sta2x11_n_mfd_plat_devs, +}; + +extern u32 +__sta2x11_mfd_mask(struct pci_dev *, u32, u32, u32, enum sta2x11_mfd_plat_dev); + /* * The MFD PCI block includes the GPIO peripherals and other register blocks. * For GPIO, we have 32*4 bits (I use "gsta" for "gpio sta2x11".) @@ -182,7 +196,11 @@ struct sta2x11_gpio_pdata { * The APB bridge has its own registers, needed by our users as well. * They are accessed with the following read/mask/write function. */ -u32 sta2x11_apbreg_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val); +static inline u32 +sta2x11_apbreg_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) +{ + return __sta2x11_mfd_mask(pdev, reg, mask, val, sta2x11_apbreg); +} /* CAN and MLB */ #define APBREG_BSR 0x00 /* Bridge Status Reg */ @@ -211,7 +229,11 @@ u32 sta2x11_apbreg_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val); * The system controller has its own registers. Some of these are accessed * by out users as well, using the following read/mask/write/function */ -u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val); +static inline +u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) +{ + return __sta2x11_mfd_mask(pdev, reg, mask, val, sta2x11_sctl); +} #define SCTL_SCCTL 0x00 /* System controller control register */ #define SCTL_ARMCFG 0x04 /* ARM configuration register */ @@ -321,4 +343,134 @@ u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val); #define SCTL_SCPEREN1_I2C3 (1 << 16) #define SCTL_SCPEREN1_USB_PHY (1 << 17) +/* + * APB-SOC registers + */ +static inline +u32 sta2x11_apb_soc_regs_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) +{ + return __sta2x11_mfd_mask(pdev, reg, mask, val, sta2x11_apb_soc_regs); +} + +#define PCIE_EP1_FUNC3_0_INTR_REG 0x000 +#define PCIE_EP1_FUNC7_4_INTR_REG 0x004 +#define PCIE_EP2_FUNC3_0_INTR_REG 0x008 +#define PCIE_EP2_FUNC7_4_INTR_REG 0x00c +#define PCIE_EP3_FUNC3_0_INTR_REG 0x010 +#define PCIE_EP3_FUNC7_4_INTR_REG 0x014 +#define PCIE_EP4_FUNC3_0_INTR_REG 0x018 +#define PCIE_EP4_FUNC7_4_INTR_REG 0x01c +#define PCIE_INTR_ENABLE0_REG 0x020 +#define PCIE_INTR_ENABLE1_REG 0x024 +#define PCIE_EP1_FUNC_TC_REG 0x028 +#define PCIE_EP2_FUNC_TC_REG 0x02c +#define PCIE_EP3_FUNC_TC_REG 0x030 +#define PCIE_EP4_FUNC_TC_REG 0x034 +#define PCIE_EP1_FUNC_F_REG 0x038 +#define PCIE_EP2_FUNC_F_REG 0x03c +#define PCIE_EP3_FUNC_F_REG 0x040 +#define PCIE_EP4_FUNC_F_REG 0x044 +#define PCIE_PAB_AMBA_SW_RST_REG 0x048 +#define PCIE_PM_STATUS_0_PORT_0_4 0x04c +#define PCIE_PM_STATUS_7_0_EP1 0x050 +#define PCIE_PM_STATUS_7_0_EP2 0x054 +#define PCIE_PM_STATUS_7_0_EP3 0x058 +#define PCIE_PM_STATUS_7_0_EP4 0x05c +#define PCIE_DEV_ID_0_EP1_REG 0x060 +#define PCIE_CC_REV_ID_0_EP1_REG 0x064 +#define PCIE_DEV_ID_1_EP1_REG 0x068 +#define PCIE_CC_REV_ID_1_EP1_REG 0x06c +#define PCIE_DEV_ID_2_EP1_REG 0x070 +#define PCIE_CC_REV_ID_2_EP1_REG 0x074 +#define PCIE_DEV_ID_3_EP1_REG 0x078 +#define PCIE_CC_REV_ID_3_EP1_REG 0x07c +#define PCIE_DEV_ID_4_EP1_REG 0x080 +#define PCIE_CC_REV_ID_4_EP1_REG 0x084 +#define PCIE_DEV_ID_5_EP1_REG 0x088 +#define PCIE_CC_REV_ID_5_EP1_REG 0x08c +#define PCIE_DEV_ID_6_EP1_REG 0x090 +#define PCIE_CC_REV_ID_6_EP1_REG 0x094 +#define PCIE_DEV_ID_7_EP1_REG 0x098 +#define PCIE_CC_REV_ID_7_EP1_REG 0x09c +#define PCIE_DEV_ID_0_EP2_REG 0x0a0 +#define PCIE_CC_REV_ID_0_EP2_REG 0x0a4 +#define PCIE_DEV_ID_1_EP2_REG 0x0a8 +#define PCIE_CC_REV_ID_1_EP2_REG 0x0ac +#define PCIE_DEV_ID_2_EP2_REG 0x0b0 +#define PCIE_CC_REV_ID_2_EP2_REG 0x0b4 +#define PCIE_DEV_ID_3_EP2_REG 0x0b8 +#define PCIE_CC_REV_ID_3_EP2_REG 0x0bc +#define PCIE_DEV_ID_4_EP2_REG 0x0c0 +#define PCIE_CC_REV_ID_4_EP2_REG 0x0c4 +#define PCIE_DEV_ID_5_EP2_REG 0x0c8 +#define PCIE_CC_REV_ID_5_EP2_REG 0x0cc +#define PCIE_DEV_ID_6_EP2_REG 0x0d0 +#define PCIE_CC_REV_ID_6_EP2_REG 0x0d4 +#define PCIE_DEV_ID_7_EP2_REG 0x0d8 +#define PCIE_CC_REV_ID_7_EP2_REG 0x0dC +#define PCIE_DEV_ID_0_EP3_REG 0x0e0 +#define PCIE_CC_REV_ID_0_EP3_REG 0x0e4 +#define PCIE_DEV_ID_1_EP3_REG 0x0e8 +#define PCIE_CC_REV_ID_1_EP3_REG 0x0ec +#define PCIE_DEV_ID_2_EP3_REG 0x0f0 +#define PCIE_CC_REV_ID_2_EP3_REG 0x0f4 +#define PCIE_DEV_ID_3_EP3_REG 0x0f8 +#define PCIE_CC_REV_ID_3_EP3_REG 0x0fc +#define PCIE_DEV_ID_4_EP3_REG 0x100 +#define PCIE_CC_REV_ID_4_EP3_REG 0x104 +#define PCIE_DEV_ID_5_EP3_REG 0x108 +#define PCIE_CC_REV_ID_5_EP3_REG 0x10c +#define PCIE_DEV_ID_6_EP3_REG 0x110 +#define PCIE_CC_REV_ID_6_EP3_REG 0x114 +#define PCIE_DEV_ID_7_EP3_REG 0x118 +#define PCIE_CC_REV_ID_7_EP3_REG 0x11c +#define PCIE_DEV_ID_0_EP4_REG 0x120 +#define PCIE_CC_REV_ID_0_EP4_REG 0x124 +#define PCIE_DEV_ID_1_EP4_REG 0x128 +#define PCIE_CC_REV_ID_1_EP4_REG 0x12c +#define PCIE_DEV_ID_2_EP4_REG 0x130 +#define PCIE_CC_REV_ID_2_EP4_REG 0x134 +#define PCIE_DEV_ID_3_EP4_REG 0x138 +#define PCIE_CC_REV_ID_3_EP4_REG 0x13c +#define PCIE_DEV_ID_4_EP4_REG 0x140 +#define PCIE_CC_REV_ID_4_EP4_REG 0x144 +#define PCIE_DEV_ID_5_EP4_REG 0x148 +#define PCIE_CC_REV_ID_5_EP4_REG 0x14c +#define PCIE_DEV_ID_6_EP4_REG 0x150 +#define PCIE_CC_REV_ID_6_EP4_REG 0x154 +#define PCIE_DEV_ID_7_EP4_REG 0x158 +#define PCIE_CC_REV_ID_7_EP4_REG 0x15c +#define PCIE_SUBSYS_VEN_ID_REG 0x160 +#define PCIE_COMMON_CLOCK_CONFIG_0_4_0 0x164 +#define PCIE_MIPHYP_SSC_EN_REG 0x168 +#define PCIE_MIPHYP_ADDR_REG 0x16c +#define PCIE_L1_ASPM_READY_REG 0x170 +#define PCIE_EXT_CFG_RDY_REG 0x174 +#define PCIE_SoC_INT_ROUTER_STATUS0_REG 0x178 +#define PCIE_SoC_INT_ROUTER_STATUS1_REG 0x17c +#define PCIE_SoC_INT_ROUTER_STATUS2_REG 0x180 +#define PCIE_SoC_INT_ROUTER_STATUS3_REG 0x184 +#define DMA_IP_CTRL_REG 0x324 +#define DISP_BRIDGE_PU_PD_CTRL_REG 0x328 +#define VIP_PU_PD_CTRL_REG 0x32c +#define USB_MLB_PU_PD_CTRL_REG 0x330 +#define SDIO_PU_PD_MISCFUNC_CTRL_REG1 0x334 +#define SDIO_PU_PD_MISCFUNC_CTRL_REG2 0x338 +#define UART_PU_PD_CTRL_REG 0x33c +#define ARM_Lock 0x340 +#define SYS_IO_CHAR_REG1 0x344 +#define SYS_IO_CHAR_REG2 0x348 +#define SATA_CORE_ID_REG 0x34c +#define SATA_CTRL_REG 0x350 +#define I2C_HSFIX_MISC_REG 0x354 +#define SPARE2_RESERVED 0x358 +#define SPARE3_RESERVED 0x35c +#define MASTER_LOCK_REG 0x368 +#define SYSTEM_CONFIG_STATUS_REG 0x36c +#define MSP_CLK_CTRL_REG 0x39c +#define COMPENSATION_REG1 0x3c4 +#define COMPENSATION_REG2 0x3c8 +#define COMPENSATION_REG3 0x3cc +#define TEST_CTL_REG 0x3d0 + #endif /* __STA2X11_MFD_H */ -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-12 10:22 ` [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code ciminaghi @ 2012-09-25 19:20 ` Mark Brown 2012-09-26 14:56 ` Davide Ciminaghi 0 siblings, 1 reply; 24+ messages in thread From: Mark Brown @ 2012-09-25 19:20 UTC (permalink / raw) To: ciminaghi; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 12, 2012 at 12:22:47PM +0200, ciminaghi@gnudd.com wrote: > From: Davide Ciminaghi <ciminaghi@gnudd.com> > > A driver for the apb-soc registers is needed by the clock > infrastructure code to configure and control clocks on the sta2x11 > chip. > Since some of the functions in sta2x11-mfd.c were almost identical > for the two existing platform devices, the following changes > have been performed to avoid further code duplication while > adding the apb-soc-regs driver: Glancing at the diff here this looks a lot like regmap-mmio... not sure if it is or not, though. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-25 19:20 ` Mark Brown @ 2012-09-26 14:56 ` Davide Ciminaghi 2012-09-26 15:04 ` Mark Brown 0 siblings, 1 reply; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-26 14:56 UTC (permalink / raw) To: Mark Brown; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Tue, Sep 25, 2012 at 08:20:48PM +0100, Mark Brown wrote: > On Wed, Sep 12, 2012 at 12:22:47PM +0200, ciminaghi@gnudd.com wrote: > > From: Davide Ciminaghi <ciminaghi@gnudd.com> > > > > A driver for the apb-soc registers is needed by the clock > > infrastructure code to configure and control clocks on the sta2x11 > > chip. > > Since some of the functions in sta2x11-mfd.c were almost identical > > for the two existing platform devices, the following changes > > have been performed to avoid further code duplication while > > adding the apb-soc-regs driver: > > Glancing at the diff here this looks a lot like regmap-mmio... not sure > if it is or not, though. that would be ideal, but I also need to deal with the common clock framework, which doesn't support regmap: clk_register_divider(), clk_register_mux(), clk_register_gate(), all take a void __iomem * to access clock control registers, so as far as I can understand we can't use regmap, unless of course we convert the common clock framework to regmap. Thanks Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-26 14:56 ` Davide Ciminaghi @ 2012-09-26 15:04 ` Mark Brown 2012-09-26 16:31 ` Davide Ciminaghi 0 siblings, 1 reply; 24+ messages in thread From: Mark Brown @ 2012-09-26 15:04 UTC (permalink / raw) To: Davide Ciminaghi; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 26, 2012 at 04:56:15PM +0200, Davide Ciminaghi wrote: > On Tue, Sep 25, 2012 at 08:20:48PM +0100, Mark Brown wrote: > > Glancing at the diff here this looks a lot like regmap-mmio... not sure > > if it is or not, though. > that would be ideal, but I also need to deal with the common clock framework, > which doesn't support regmap: clk_register_divider(), clk_register_mux(), > clk_register_gate(), all take a void __iomem * to access clock control > registers, so as far as I can understand we can't use regmap, unless of > course we convert the common clock framework to regmap. The two should be able to coexist happily, regmap-mmio requires the caller to do the actual mapping and so long as you don't cache the relevant registers the two shouldn't interfere with each other. Alternatively, and more generally usefully, we could implement regmap based common clock helpers. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-26 15:04 ` Mark Brown @ 2012-09-26 16:31 ` Davide Ciminaghi 2012-09-26 16:49 ` Mark Brown 0 siblings, 1 reply; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-26 16:31 UTC (permalink / raw) To: Mark Brown; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 26, 2012 at 04:04:29PM +0100, Mark Brown wrote: > On Wed, Sep 26, 2012 at 04:56:15PM +0200, Davide Ciminaghi wrote: > > On Tue, Sep 25, 2012 at 08:20:48PM +0100, Mark Brown wrote: > > > > Glancing at the diff here this looks a lot like regmap-mmio... not sure > > > if it is or not, though. > > > that would be ideal, but I also need to deal with the common clock framework, > > which doesn't support regmap: clk_register_divider(), clk_register_mux(), > > clk_register_gate(), all take a void __iomem * to access clock control > > registers, so as far as I can understand we can't use regmap, unless of > > course we convert the common clock framework to regmap. > > The two should be able to coexist happily, regmap-mmio requires the > caller to do the actual mapping and so long as you don't cache the > relevant registers the two shouldn't interfere with each other. > Well, it just sounded not so clean to me having two ways for accessing the same registers. The caller would be sta2x11-mfd in our case, while clock registers are used by the common clock framework, so sta2x11-mfd should still need to export some void __iomem * to make the clock framework happy, while other potential users could go via regmap. Oh, and there's another problem (I'm looking at the code right now, I had forgotten about this): the clock framework also asks for a spinlock_t *. Regmap has its own spinlock (or mutex), and I don't think it would be a good idea trying to export it. So, as far as I can understand at present, the best thing to do would be adding regmap support to the clock framework, which wouldn't be trivial because I guess we should keep compatibility with the present clk_register_... api Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-26 16:31 ` Davide Ciminaghi @ 2012-09-26 16:49 ` Mark Brown 2012-09-27 8:39 ` Davide Ciminaghi ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Mark Brown @ 2012-09-26 16:49 UTC (permalink / raw) To: Davide Ciminaghi; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 26, 2012 at 06:31:45PM +0200, Davide Ciminaghi wrote: > Oh, and there's another problem (I'm looking at the code right now, I had > forgotten about this): the clock framework also asks for a spinlock_t *. > Regmap has its own spinlock (or mutex), and I don't think it would be a > good idea trying to export it. Why is this a problem? Nested spinlocks are perfectly fine. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-26 16:49 ` Mark Brown @ 2012-09-27 8:39 ` Davide Ciminaghi 2012-09-27 8:56 ` Davide Ciminaghi 2012-09-27 13:41 ` Davide Ciminaghi 2 siblings, 0 replies; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-27 8:39 UTC (permalink / raw) To: Mark Brown; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 26, 2012 at 05:49:33PM +0100, Mark Brown wrote: > On Wed, Sep 26, 2012 at 06:31:45PM +0200, Davide Ciminaghi wrote: > > > Oh, and there's another problem (I'm looking at the code right now, I had > > forgotten about this): the clock framework also asks for a spinlock_t *. > > Regmap has its own spinlock (or mutex), and I don't think it would be a > > good idea trying to export it. > > Why is this a problem? Nested spinlocks are perfectly fine. sorry, maybe my understanding of regmap is poor, but the problem I see is that the regmap spinlock belongs to struct regmap, which is defined in drivers/regmap/internal.h, so it is not visible to the caller(s) (sta2x11-mfd in this case). My interpretation is that the reason for hiding struct regmap (and the spinlock as a consequence) has to do with what object-oriented people call "information hiding", so exporting the spinlock address would somehow break the design. We could add some regmap_get_lock_address() and read the spinlock address from sta2x11-mfd, but then to be generic we should keep into account that a mutex could be used by some bus, so the function prototype could for example look like this: void regmap_get_lock_address(struct regmap *, spinlock_t *s, struct mutex *m); so, if a spinlock is used s would be filled with the spinlock's address (and m set to NULL), while if a mutex is used, m would be set to the address of the mutex (and s set to NULL). My personal opinion is that introducing such a function would expose too many implementation details to regmap users, but if you think it is ok, this solution would of course be very simple. Thanks Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-26 16:49 ` Mark Brown 2012-09-27 8:39 ` Davide Ciminaghi @ 2012-09-27 8:56 ` Davide Ciminaghi 2012-09-27 13:41 ` Davide Ciminaghi 2 siblings, 0 replies; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-27 8:56 UTC (permalink / raw) To: Mark Brown; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 26, 2012 at 05:49:33PM +0100, Mark Brown wrote: > On Wed, Sep 26, 2012 at 06:31:45PM +0200, Davide Ciminaghi wrote: > > > Oh, and there's another problem (I'm looking at the code right now, I had > > forgotten about this): the clock framework also asks for a spinlock_t *. > > Regmap has its own spinlock (or mutex), and I don't think it would be a > > good idea trying to export it. > > Why is this a problem? Nested spinlocks are perfectly fine. sorry, forget my last reply, I had not enough sleep tonight. That's ok, I will use nested spinlocks. Thanks Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-26 16:49 ` Mark Brown 2012-09-27 8:39 ` Davide Ciminaghi 2012-09-27 8:56 ` Davide Ciminaghi @ 2012-09-27 13:41 ` Davide Ciminaghi 2012-09-27 13:49 ` Mark Brown 2012-09-27 14:13 ` Alan Cox 2 siblings, 2 replies; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-27 13:41 UTC (permalink / raw) To: Mark Brown; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Wed, Sep 26, 2012 at 05:49:33PM +0100, Mark Brown wrote: > On Wed, Sep 26, 2012 at 06:31:45PM +0200, Davide Ciminaghi wrote: > > > Oh, and there's another problem (I'm looking at the code right now, I had > > forgotten about this): the clock framework also asks for a spinlock_t *. > > Regmap has its own spinlock (or mutex), and I don't think it would be a > > good idea trying to export it. > > Why is this a problem? Nested spinlocks are perfectly fine. after some cups of coffee, I tried thinking about this subject again, and here's what I found: as far as I know, nested locks are fine provided that you always take them in the same order and release them in the opposite order (lock A, lock B, unlock B, unlock A). So my conclusion is that nested spinlocks require potential regmap users of sta2x11 registers to take the sta2x11-mfd spinlock first. The pattern would be (sctl registers for instance): spinlock_t *mfd_lock; void *addr; /* Read sta2x11-mfd spinlock address for sctl registers and put it into mfd_lock */ sta2x11_mfd_get_regs_data(pdev, sta2x11_sctl, &addr, &mfd_lock); ...... /* Take the outer lock */ spin_lock(mfd_lock); /* Take the inner lock, do whatever you need to do and release the inner lock */ regmap_...(); /* Release the outer lock */ spin_unlock(mfd_lock); The other way around (taking the regmap spinlock as the "outer" one) would imply at least exporting the regmap spinlock/mutex address or making the regmap.lock/unlock function pointers public somehow, not to mention the fact that, for instance, the clock framework helpers should be modified to deal with nested locking. If the above is correct, I'm not sure we'd gain a lot from regmap, because we'd still need some sta2x11 extra code around regmap calls. Maybe there's another solution: what about adding a couple of function pointers (lock, unlock) to struct regmap_config ? If they're set to NULL, everything works as usual. If they're not NULL, regmap uses such functions to do locking and unlocking instead of the usual ones (and regmap_bus.fast_io is ignored). I think this wouldn't hurt existing regmap users and allow sta2x11-mfd to have just one spinlock for everything, no need for nested locking this way. Thanks and regards Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-27 13:41 ` Davide Ciminaghi @ 2012-09-27 13:49 ` Mark Brown 2012-09-28 8:43 ` Davide Ciminaghi 2012-09-27 14:13 ` Alan Cox 1 sibling, 1 reply; 24+ messages in thread From: Mark Brown @ 2012-09-27 13:49 UTC (permalink / raw) To: Davide Ciminaghi; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Thu, Sep 27, 2012 at 03:41:10PM +0200, Davide Ciminaghi wrote: > Maybe there's another solution: what about adding a couple of function > pointers (lock, unlock) to struct regmap_config ? If they're set to NULL, > everything works as usual. If they're not NULL, regmap uses such functions > to do locking and unlocking instead of the usual ones (and regmap_bus.fast_io > is ignored). I think this wouldn't hurt existing regmap users and allow > sta2x11-mfd to have just one spinlock for everything, no need for nested > locking this way. That might work, yes, and would be generally useful I think. Or we could add regmap based versions of the clock utilities (which would also be useful anyway). Or both. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-27 13:49 ` Mark Brown @ 2012-09-28 8:43 ` Davide Ciminaghi 2012-09-28 10:52 ` Mark Brown 0 siblings, 1 reply; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-28 8:43 UTC (permalink / raw) To: Mark Brown; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Thu, Sep 27, 2012 at 02:49:19PM +0100, Mark Brown wrote: > On Thu, Sep 27, 2012 at 03:41:10PM +0200, Davide Ciminaghi wrote: > > > Maybe there's another solution: what about adding a couple of function > > pointers (lock, unlock) to struct regmap_config ? If they're set to NULL, > > everything works as usual. If they're not NULL, regmap uses such functions > > to do locking and unlocking instead of the usual ones (and regmap_bus.fast_io > > is ignored). I think this wouldn't hurt existing regmap users and allow > > sta2x11-mfd to have just one spinlock for everything, no need for nested > > locking this way. > > That might work, yes, and would be generally useful I think. Or we > could add regmap based versions of the clock utilities (which would also > be useful anyway). Or both. I have a (first, incomplete but working) clock framework implementation for the sta2x11 ready, but can't submit it until these sta2x11-mfd patches have been accepted. And then of course many of the drivers we need on sta2x11 depend on the clock framework (they're amba drivers). So, since sta2x11-mfd is blocking much of our work, I would proceed like this: 1. Regmap patch for overriding lock/unlock functions 2. Regmap based reimplementation of sta2x11-mfd (using sta2x11 special lock/unlock functions). 3. Submission of sta2x11 common clock framework as is (using the current version of the clock framework helpers, no regmap). 4. Patches for some amba drivers to get them to work on sta2x11. 5. Regmap related patches for the common clock framework. Does this sound reasonable ? Thanks and regards Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-28 8:43 ` Davide Ciminaghi @ 2012-09-28 10:52 ` Mark Brown 0 siblings, 0 replies; 24+ messages in thread From: Mark Brown @ 2012-09-28 10:52 UTC (permalink / raw) To: Davide Ciminaghi; +Cc: sameo, rubini, giancarlo.asnaghi, linux-kernel On Fri, Sep 28, 2012 at 10:43:03AM +0200, Davide Ciminaghi wrote: > 1. Regmap patch for overriding lock/unlock functions > 2. Regmap based reimplementation of sta2x11-mfd (using sta2x11 special > lock/unlock functions). > 3. Submission of sta2x11 common clock framework as is (using the current > version of the clock framework helpers, no regmap). > 4. Patches for some amba drivers to get them to work on sta2x11. > 5. Regmap related patches for the common clock framework. > Does this sound reasonable ? Makes sense. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-27 13:41 ` Davide Ciminaghi 2012-09-27 13:49 ` Mark Brown @ 2012-09-27 14:13 ` Alan Cox 2012-09-28 15:29 ` Davide Ciminaghi 1 sibling, 1 reply; 24+ messages in thread From: Alan Cox @ 2012-09-27 14:13 UTC (permalink / raw) To: Davide Ciminaghi Cc: Mark Brown, sameo, rubini, giancarlo.asnaghi, linux-kernel > as far as I know, nested locks are fine provided that you always take them in > the same order and release them in the opposite order (lock A, lock B, > unlock B, unlock A). So my conclusion is that nested spinlocks require > potential regmap users of sta2x11 registers to take the sta2x11-mfd spinlock > first. The pattern would be (sctl registers for instance): The release order does not matter. Taking AB and releasing AB or BA is fine. Taking AB and dropping B and retaking B is fine. Taking AB and somewhere else taking BA is not. There are performance reasons in some cases why taking AB releasing A is best with locks, but thats generally with sleepable locks. It's a bit more subtle because you often have other interactions. In particular people often come unstuck on del_timer_sync or waiting for workqueues and IRQs to finish (even in free_irq) while holding a lock that the handler needs to take. Alan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code 2012-09-27 14:13 ` Alan Cox @ 2012-09-28 15:29 ` Davide Ciminaghi 0 siblings, 0 replies; 24+ messages in thread From: Davide Ciminaghi @ 2012-09-28 15:29 UTC (permalink / raw) To: Alan Cox; +Cc: Mark Brown, sameo, rubini, giancarlo.asnaghi, linux-kernel On Thu, Sep 27, 2012 at 03:13:14PM +0100, Alan Cox wrote: > > as far as I know, nested locks are fine provided that you always take them in > > the same order and release them in the opposite order (lock A, lock B, > > unlock B, unlock A). So my conclusion is that nested spinlocks require > > potential regmap users of sta2x11 registers to take the sta2x11-mfd spinlock > > first. The pattern would be (sctl registers for instance): > > The release order does not matter. Taking AB and releasing AB or BA is > fine. Taking AB and dropping B and retaking B is fine. Taking AB and > somewhere else taking BA is not. There are performance reasons in some > cases why taking AB releasing A is best with locks, but thats generally > with sleepable locks. > thanks, I'd never really thought about the release pattern actually. In this particular case, though, B is inside the regmap code and is taken and released when a regmap call happens, so I think AB necessarily implies BA. For instance: lock(A); regmap_read(...); /* Lock/unlock B */ unlock(A); Thanks Davide ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi 2012-09-12 10:22 ` [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-12 10:22 ` [PATCH 3/8] sta2x11-mfd : use defines for platform devices' names ciminaghi ` (6 subsequent siblings) 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> A couple of predefined clocks (mux and gated) need to be initialized with the virtual address of the clock's controlling register and the address of a spinlock used to protect against races. This function exports such data for all the mfd cells. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 22 ++++++++++++++++++++++ include/linux/mfd/sta2x11-mfd.h | 5 +++++ 2 files changed, 27 insertions(+), 0 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index 5fbd044..905b184 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -127,6 +127,28 @@ u32 __sta2x11_mfd_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val, } EXPORT_SYMBOL(__sta2x11_mfd_mask); +int sta2x11_mfd_get_regs_data(struct platform_device *dev, + enum sta2x11_mfd_plat_dev index, + void __iomem **regs, + spinlock_t **lock) +{ + struct pci_dev *pdev = *(struct pci_dev **)(dev->dev.platform_data); + struct sta2x11_mfd *mfd; + + if (!pdev) + return -ENODEV; + mfd = sta2x11_mfd_find(pdev); + if (!mfd) + return -ENODEV; + if (index >= sta2x11_n_mfd_plat_devs) + return -ENODEV; + *regs = mfd->regs[index]; + *lock = &mfd->lock[index]; + pr_debug("%s %d *regs = %p\n", __func__, __LINE__, *regs); + return *regs ? 0 : -ENODEV; +} +EXPORT_SYMBOL(sta2x11_mfd_get_regs_data); + /* Two debugfs files, for our registers (FIXME: one instance only) */ #define REG(regname) {.name = #regname, .offset = SCTL_ ## regname} static struct debugfs_reg32 sta2x11_sctl_regs[] = { diff --git a/include/linux/mfd/sta2x11-mfd.h b/include/linux/mfd/sta2x11-mfd.h index 4d85879..9ba6127 100644 --- a/include/linux/mfd/sta2x11-mfd.h +++ b/include/linux/mfd/sta2x11-mfd.h @@ -473,4 +473,9 @@ u32 sta2x11_apb_soc_regs_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) #define COMPENSATION_REG3 0x3cc #define TEST_CTL_REG 0x3d0 +extern int sta2x11_mfd_get_regs_data(struct platform_device *pdev, + enum sta2x11_mfd_plat_dev index, + void __iomem **regs, + spinlock_t **lock); + #endif /* __STA2X11_MFD_H */ -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/8] sta2x11-mfd : use defines for platform devices' names 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi 2012-09-12 10:22 ` [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code ciminaghi 2012-09-12 10:22 ` [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-12 10:22 ` [PATCH 4/8] sta2x11-mfd : only add sta2x11_mfd if it hasn't already been added ciminaghi ` (5 subsequent siblings) 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> Since there are now many sta2x11-mfd platform devices, using defines for their names looks like a better solution. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 42 ++++++++++++++++++++------------------ include/linux/mfd/sta2x11-mfd.h | 8 +++++++ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index 905b184..a9eeba1 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -258,9 +258,9 @@ static struct debugfs_regset32 *sta2x11_mfd_regset[sta2x11_n_mfd_plat_devs] = { }; static const char *sta2x11_mfd_names[sta2x11_n_mfd_plat_devs] = { - [sta2x11_sctl] = "sta2x11-sctl", - [sta2x11_apbreg] = "sta2x11-apbreg", - [sta2x11_apb_soc_regs] = "sta2x11-apb-soc-regs", + [sta2x11_sctl] = STA2X11_MFD_SCTL_NAME, + [sta2x11_apbreg] = STA2X11_MFD_APBREG_NAME, + [sta2x11_apb_soc_regs] = STA2X11_MFD_APB_SOC_REGS_NAME, }; /* Probe for the three platform devices */ @@ -316,7 +316,7 @@ static int sta2x11_apb_soc_regs_probe(struct platform_device *dev) /* The three platform drivers */ static struct platform_driver sta2x11_sctl_platform_driver = { .driver = { - .name = "sta2x11-sctl", + .name = STA2X11_MFD_SCTL_NAME, .owner = THIS_MODULE, }, .probe = sta2x11_sctl_probe, @@ -330,7 +330,7 @@ static int __init sta2x11_sctl_init(void) static struct platform_driver sta2x11_platform_driver = { .driver = { - .name = "sta2x11-apbreg", + .name = STA2X11_MFD_APBREG_NAME, .owner = THIS_MODULE, }, .probe = sta2x11_apbreg_probe, @@ -344,7 +344,7 @@ static int __init sta2x11_apbreg_init(void) static struct platform_driver sta2x11_apb_soc_regs_platform_driver = { .driver = { - .name = "sta2x11-apb-soc-regs", + .name = STA2X11_MFD_APB_SOC_REGS_NAME, .owner = THIS_MODULE, }, .probe = sta2x11_apb_soc_regs_probe, @@ -385,38 +385,40 @@ enum mfd0_bar1_cells { static const __devinitconst struct resource gpio_resources[] = { { - .name = "sta2x11_gpio", /* 4 consecutive cells, 1 driver */ + /* 4 consecutive cells, 1 driver */ + .name = STA2X11_MFD_GPIO_NAME, .start = 0, .end = (4 * 4096) - 1, .flags = IORESOURCE_MEM, } }; static const __devinitconst struct resource sctl_resources[] = { - CELL_4K("sta2x11-sctl", STA2X11_SCTL), + CELL_4K(STA2X11_MFD_SCTL_NAME, STA2X11_SCTL), }; static const __devinitconst struct resource scr_resources[] = { - CELL_4K("sta2x11-scr", STA2X11_SCR), + CELL_4K(STA2X11_MFD_SCR_NAME, STA2X11_SCR), }; static const __devinitconst struct resource time_resources[] = { - CELL_4K("sta2x11-time", STA2X11_TIME), + CELL_4K(STA2X11_MFD_TIME_NAME, STA2X11_TIME), }; static const __devinitconst struct resource apbreg_resources[] = { - CELL_4K("sta2x11-apbreg", STA2X11_APBREG), + CELL_4K(STA2X11_MFD_APBREG_NAME, STA2X11_APBREG), }; #define DEV(_name, _r) \ { .name = _name, .num_resources = ARRAY_SIZE(_r), .resources = _r, } static __devinitdata struct mfd_cell sta2x11_mfd0_bar0[] = { - DEV("sta2x11-gpio", gpio_resources), /* offset 0: we add pdata later */ - DEV("sta2x11-sctl", sctl_resources), - DEV("sta2x11-scr", scr_resources), - DEV("sta2x11-time", time_resources), + /* offset 0: we add pdata later */ + DEV(STA2X11_MFD_GPIO_NAME, gpio_resources), + DEV(STA2X11_MFD_SCTL_NAME, sctl_resources), + DEV(STA2X11_MFD_SCR_NAME, scr_resources), + DEV(STA2X11_MFD_TIME_NAME, time_resources), }; static __devinitdata struct mfd_cell sta2x11_mfd0_bar1[] = { - DEV("sta2x11-apbreg", apbreg_resources), + DEV(STA2X11_MFD_APBREG_NAME, apbreg_resources), }; /* Mfd 1 devices */ @@ -432,19 +434,19 @@ enum mfd1_bar1_cells { }; static const __devinitconst struct resource vic_resources[] = { - CELL_4K("sta2x11-vic", STA2X11_VIC), + CELL_4K(STA2X11_MFD_VIC_NAME, STA2X11_VIC), }; static const __devinitconst struct resource apb_soc_regs_resources[] = { - CELL_4K("sta2x11-apb-soc-regs", STA2X11_APB_SOC_REGS), + CELL_4K(STA2X11_MFD_APB_SOC_REGS_NAME, STA2X11_APB_SOC_REGS), }; static __devinitdata struct mfd_cell sta2x11_mfd1_bar0[] = { - DEV("sta2x11-vic", vic_resources), + DEV(STA2X11_MFD_VIC_NAME, vic_resources), }; static __devinitdata struct mfd_cell sta2x11_mfd1_bar1[] = { - DEV("sta2x11-apb-soc-regs", apb_soc_regs_resources), + DEV(STA2X11_MFD_APB_SOC_REGS_NAME, apb_soc_regs_resources), }; diff --git a/include/linux/mfd/sta2x11-mfd.h b/include/linux/mfd/sta2x11-mfd.h index 9ba6127..9de20fd 100644 --- a/include/linux/mfd/sta2x11-mfd.h +++ b/include/linux/mfd/sta2x11-mfd.h @@ -37,6 +37,14 @@ enum sta2x11_mfd_plat_dev { sta2x11_n_mfd_plat_devs, }; +#define STA2X11_MFD_SCTL_NAME "sta2x11-sctl" +#define STA2X11_MFD_GPIO_NAME "sta2x11-gpio" +#define STA2X11_MFD_SCR_NAME "sta2x11-scr" +#define STA2X11_MFD_TIME_NAME "sta2x11-time" +#define STA2X11_MFD_APBREG_NAME "sta2x11-apbreg" +#define STA2X11_MFD_APB_SOC_REGS_NAME "sta2x11-apb-soc-regs" +#define STA2X11_MFD_VIC_NAME "sta2x11-vic" + extern u32 __sta2x11_mfd_mask(struct pci_dev *, u32, u32, u32, enum sta2x11_mfd_plat_dev); -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/8] sta2x11-mfd : only add sta2x11_mfd if it hasn't already been added 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi ` (2 preceding siblings ...) 2012-09-12 10:22 ` [PATCH 3/8] sta2x11-mfd : use defines for platform devices' names ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-12 10:22 ` [PATCH 5/8] sta2x11-mfd : platform probe: don't mind about gpio platform data ciminaghi ` (4 subsequent siblings) 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> The pci probe method is called twice now, so we have to call sta2x11_mfd_add() only once to avoid a -EBUSY error. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index a9eeba1..82fe356 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -561,7 +561,8 @@ static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev, sta2x11_mfd_setup(pdev, setup_data); /* Record this pdev before mfd_add_devices: their probe looks for it */ - sta2x11_mfd_add(pdev, GFP_ATOMIC); + if (!sta2x11_mfd_find(pdev)) + sta2x11_mfd_add(pdev, GFP_ATOMIC); /* Just 2 bars for all mfd's at present */ for (i = 0; i < 2; i++) { -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/8] sta2x11-mfd : platform probe: don't mind about gpio platform data 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi ` (3 preceding siblings ...) 2012-09-12 10:22 ` [PATCH 4/8] sta2x11-mfd : only add sta2x11_mfd if it hasn't already been added ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-12 10:22 ` [PATCH 6/8] sta2x11-mfd : use one lock per device instead of one lock per mfd ciminaghi ` (3 subsequent siblings) 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> The gpio platform driver will take care of its platform data, let's not do any checks here. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 11 ----------- 1 files changed, 0 insertions(+), 11 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index 82fe356..88bc8d5 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -529,7 +529,6 @@ static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev, { int err, i; struct sta2x11_mfd_setup_data *setup_data; - struct sta2x11_gpio_pdata *gpio_data; dev_info(&pdev->dev, "%s\n", __func__); @@ -547,16 +546,6 @@ static int __devinit sta2x11_mfd_probe(struct pci_dev *pdev, &mfd_setup_data[STA2X11_MFD0] : &mfd_setup_data[STA2X11_MFD1]; - /* Read gpio config data as pci device's platform data */ - gpio_data = dev_get_platdata(&pdev->dev); - if (!gpio_data) - dev_warn(&pdev->dev, "no gpio configuration\n"); - - dev_dbg(&pdev->dev, "%s, gpio_data = %p (%p)\n", __func__, - gpio_data, &gpio_data); - dev_dbg(&pdev->dev, "%s, pdev = %p (%p)\n", __func__, - pdev, &pdev); - /* platform data is the pci device for all of them */ sta2x11_mfd_setup(pdev, setup_data); -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/8] sta2x11-mfd : use one lock per device instead of one lock per mfd. 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi ` (4 preceding siblings ...) 2012-09-12 10:22 ` [PATCH 5/8] sta2x11-mfd : platform probe: don't mind about gpio platform data ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-12 10:22 ` [PATCH 7/8] sta2x11-mfd : add defines for some sta2x11 sctl registers ciminaghi ` (2 subsequent siblings) 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> The lock is used to implement atomic operations on each platform device's registers, so it looks reasonable having one lock per device instead of one common lock for all the devices belonging to the same sta2x11 instance. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index 88bc8d5..2c6f777 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -38,7 +38,7 @@ /* This describes STA2X11 MFD chip for us, we may have several */ struct sta2x11_mfd { struct sta2x11_instance *instance; - spinlock_t lock; + spinlock_t lock[sta2x11_n_mfd_plat_devs]; struct list_head list; void __iomem *regs[sta2x11_n_mfd_plat_devs]; }; @@ -70,6 +70,7 @@ static struct sta2x11_mfd *sta2x11_mfd_find(struct pci_dev *pdev) static int __devinit sta2x11_mfd_add(struct pci_dev *pdev, gfp_t flags) { + int i; struct sta2x11_mfd *mfd = sta2x11_mfd_find(pdev); struct sta2x11_instance *instance; @@ -82,7 +83,8 @@ static int __devinit sta2x11_mfd_add(struct pci_dev *pdev, gfp_t flags) if (!mfd) return -ENOMEM; INIT_LIST_HEAD(&mfd->list); - spin_lock_init(&mfd->lock); + for (i = 0; i < ARRAY_SIZE(mfd->lock); i++) + spin_lock_init(&mfd->lock[i]); mfd->instance = instance; list_add(&mfd->list, &sta2x11_mfd_list); return 0; @@ -116,13 +118,13 @@ u32 __sta2x11_mfd_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val, dev_warn(&pdev->dev, ": system ctl not initialized\n"); return 0; } - spin_lock_irqsave(&mfd->lock, flags); + spin_lock_irqsave(&mfd->lock[index], flags); r = readl(regs + reg); r &= ~mask; r |= val; if (mask) writel(r, regs + reg); - spin_unlock_irqrestore(&mfd->lock, flags); + spin_unlock_irqrestore(&mfd->lock[index], flags); return r; } EXPORT_SYMBOL(__sta2x11_mfd_mask); -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/8] sta2x11-mfd : add defines for some sta2x11 sctl registers. 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi ` (5 preceding siblings ...) 2012-09-12 10:22 ` [PATCH 6/8] sta2x11-mfd : use one lock per device instead of one lock per mfd ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-12 10:22 ` [PATCH 8/8] sta2x11-mfd : add myself to copyright ciminaghi 2012-09-16 21:25 ` [PATCH 0/8 RESEND] sta2x11-mfd patches Alessandro Rubini 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> These are required for the clock infrastructure code to properly configure and control the sta2x11 PLLs. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- include/linux/mfd/sta2x11-mfd.h | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/include/linux/mfd/sta2x11-mfd.h b/include/linux/mfd/sta2x11-mfd.h index 9de20fd..2c29924 100644 --- a/include/linux/mfd/sta2x11-mfd.h +++ b/include/linux/mfd/sta2x11-mfd.h @@ -246,8 +246,29 @@ u32 sta2x11_sctl_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) #define SCTL_SCCTL 0x00 /* System controller control register */ #define SCTL_ARMCFG 0x04 /* ARM configuration register */ #define SCTL_SCPLLCTL 0x08 /* PLL control status register */ + +#define SCTL_SCPLLCTL_AUDIO_PLL_PD BIT(1) +#define SCTL_SCPLLCTL_FRAC_CONTROL BIT(3) +#define SCTL_SCPLLCTL_STRB_BYPASS BIT(6) +#define SCTL_SCPLLCTL_STRB_INPUT BIT(8) + #define SCTL_SCPLLFCTRL 0x0c /* PLL frequency control register */ + +#define SCTL_SCPLLFCTRL_AUDIO_PLL_NDIV_MASK 0xff +#define SCTL_SCPLLFCTRL_AUDIO_PLL_NDIV_SHIFT 10 +#define SCTL_SCPLLFCTRL_AUDIO_PLL_IDF_MASK 7 +#define SCTL_SCPLLFCTRL_AUDIO_PLL_IDF_SHIFT 21 +#define SCTL_SCPLLFCTRL_AUDIO_PLL_ODF_MASK 7 +#define SCTL_SCPLLFCTRL_AUDIO_PLL_ODF_SHIFT 18 +#define SCTL_SCPLLFCTRL_DITHER_DISABLE_MASK 0x03 +#define SCTL_SCPLLFCTRL_DITHER_DISABLE_SHIFT 4 + + #define SCTL_SCRESFRACT 0x10 /* PLL fractional input register */ + +#define SCTL_SCRESFRACT_MASK 0x0000ffff + + #define SCTL_SCRESCTRL1 0x14 /* Peripheral reset control 1 */ #define SCTL_SCRESXTRL2 0x18 /* Peripheral reset control 2 */ #define SCTL_SCPEREN0 0x1c /* Peripheral clock enable register 0 */ -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/8] sta2x11-mfd : add myself to copyright 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi ` (6 preceding siblings ...) 2012-09-12 10:22 ` [PATCH 7/8] sta2x11-mfd : add defines for some sta2x11 sctl registers ciminaghi @ 2012-09-12 10:22 ` ciminaghi 2012-09-16 21:25 ` [PATCH 0/8 RESEND] sta2x11-mfd patches Alessandro Rubini 8 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:22 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index 2c6f777..d5ce5e3 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -1,6 +1,6 @@ /* * Copyright (c) 2009-2011 Wind River Systems, Inc. - * Copyright (c) 2011 ST Microelectronics (Alessandro Rubini) + * Copyright (c) 2011 ST Microelectronics (Alessandro Rubini, Davide Ciminaghi) * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 0/8 RESEND] sta2x11-mfd patches 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi ` (7 preceding siblings ...) 2012-09-12 10:22 ` [PATCH 8/8] sta2x11-mfd : add myself to copyright ciminaghi @ 2012-09-16 21:25 ` Alessandro Rubini 8 siblings, 0 replies; 24+ messages in thread From: Alessandro Rubini @ 2012-09-16 21:25 UTC (permalink / raw) To: ciminaghi; +Cc: ciminaghi, sameo, giancarlo.asnaghi, linux-kernel > Davide Ciminaghi (8): For the whole set: Acked-by: Alessandro Rubini <rubini@gnudd.com> (original committer of the mfd driver) /alessandro ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/8 RESEND] sta2x11-mfd patches @ 2012-09-12 10:11 ciminaghi 2012-09-12 10:11 ` [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function ciminaghi 0 siblings, 1 reply; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:11 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> Hi, this is a resend of a patchset dated Jun 10th (see https://lkml.org/lkml/2012/6/10/188) even if some of the commits have been squashed. I'm working on the implementation of the common clock framework for the STA2X11 (aka CONNEXT) soc. To configure, enable and disable clocks, some general purpose registers (apb-soc-registers) must be accessed. Such registers can be reached through bar #1 of a multifunction pci device, which is not covered by the current version of sta2x11-mfd. This patch series adds support for the apb-soc-registers device and rearranges the sta2x11-mfd driver to avoid unnecessary code duplications and to cope with the increased number of platform devices. Davide Davide Ciminaghi (8): sta2x11-mfd : add apb-soc regs driver and factor out common code sta2x11-mfd : add sta2x11_mfd_get_regs_data() function sta2x11-mfd : use defines for platform devices' names sta2x11-mfd : only add sta2x11_mfd if it hasn't already been added sta2x11-mfd : platform probe: don't mind about gpio platform data sta2x11-mfd : use one lock per device instead of one lock per mfd. sta2x11-mfd : add defines for some sta2x11 sctl registers. sta2x11-mfd : add myself to copyright drivers/mfd/sta2x11-mfd.c | 416 ++++++++++++++++++++++++++------------- include/linux/mfd/sta2x11-mfd.h | 190 ++++++++++++++++++- 2 files changed, 470 insertions(+), 136 deletions(-) -- 1.7.9.1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function 2012-09-12 10:11 ciminaghi @ 2012-09-12 10:11 ` ciminaghi 0 siblings, 0 replies; 24+ messages in thread From: ciminaghi @ 2012-09-12 10:11 UTC (permalink / raw) To: sameo, rubini, giancarlo.asnaghi; +Cc: linux-kernel, Davide Ciminaghi From: Davide Ciminaghi <ciminaghi@gnudd.com> A couple of predefined clocks (mux and gated) need to be initialized with the virtual address of the clock's controlling register and the address of a spinlock used to protect against races. This function exports such data for all the mfd cells. Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com> --- drivers/mfd/sta2x11-mfd.c | 22 ++++++++++++++++++++++ include/linux/mfd/sta2x11-mfd.h | 5 +++++ 2 files changed, 27 insertions(+), 0 deletions(-) diff --git a/drivers/mfd/sta2x11-mfd.c b/drivers/mfd/sta2x11-mfd.c index 5fbd044..905b184 100644 --- a/drivers/mfd/sta2x11-mfd.c +++ b/drivers/mfd/sta2x11-mfd.c @@ -127,6 +127,28 @@ u32 __sta2x11_mfd_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val, } EXPORT_SYMBOL(__sta2x11_mfd_mask); +int sta2x11_mfd_get_regs_data(struct platform_device *dev, + enum sta2x11_mfd_plat_dev index, + void __iomem **regs, + spinlock_t **lock) +{ + struct pci_dev *pdev = *(struct pci_dev **)(dev->dev.platform_data); + struct sta2x11_mfd *mfd; + + if (!pdev) + return -ENODEV; + mfd = sta2x11_mfd_find(pdev); + if (!mfd) + return -ENODEV; + if (index >= sta2x11_n_mfd_plat_devs) + return -ENODEV; + *regs = mfd->regs[index]; + *lock = &mfd->lock[index]; + pr_debug("%s %d *regs = %p\n", __func__, __LINE__, *regs); + return *regs ? 0 : -ENODEV; +} +EXPORT_SYMBOL(sta2x11_mfd_get_regs_data); + /* Two debugfs files, for our registers (FIXME: one instance only) */ #define REG(regname) {.name = #regname, .offset = SCTL_ ## regname} static struct debugfs_reg32 sta2x11_sctl_regs[] = { diff --git a/include/linux/mfd/sta2x11-mfd.h b/include/linux/mfd/sta2x11-mfd.h index 4d85879..9ba6127 100644 --- a/include/linux/mfd/sta2x11-mfd.h +++ b/include/linux/mfd/sta2x11-mfd.h @@ -473,4 +473,9 @@ u32 sta2x11_apb_soc_regs_mask(struct pci_dev *pdev, u32 reg, u32 mask, u32 val) #define COMPENSATION_REG3 0x3cc #define TEST_CTL_REG 0x3d0 +extern int sta2x11_mfd_get_regs_data(struct platform_device *pdev, + enum sta2x11_mfd_plat_dev index, + void __iomem **regs, + spinlock_t **lock); + #endif /* __STA2X11_MFD_H */ -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-09-28 15:30 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-12 10:22 [PATCH 0/8 RESEND] sta2x11-mfd patches ciminaghi 2012-09-12 10:22 ` [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code ciminaghi 2012-09-25 19:20 ` Mark Brown 2012-09-26 14:56 ` Davide Ciminaghi 2012-09-26 15:04 ` Mark Brown 2012-09-26 16:31 ` Davide Ciminaghi 2012-09-26 16:49 ` Mark Brown 2012-09-27 8:39 ` Davide Ciminaghi 2012-09-27 8:56 ` Davide Ciminaghi 2012-09-27 13:41 ` Davide Ciminaghi 2012-09-27 13:49 ` Mark Brown 2012-09-28 8:43 ` Davide Ciminaghi 2012-09-28 10:52 ` Mark Brown 2012-09-27 14:13 ` Alan Cox 2012-09-28 15:29 ` Davide Ciminaghi 2012-09-12 10:22 ` [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function ciminaghi 2012-09-12 10:22 ` [PATCH 3/8] sta2x11-mfd : use defines for platform devices' names ciminaghi 2012-09-12 10:22 ` [PATCH 4/8] sta2x11-mfd : only add sta2x11_mfd if it hasn't already been added ciminaghi 2012-09-12 10:22 ` [PATCH 5/8] sta2x11-mfd : platform probe: don't mind about gpio platform data ciminaghi 2012-09-12 10:22 ` [PATCH 6/8] sta2x11-mfd : use one lock per device instead of one lock per mfd ciminaghi 2012-09-12 10:22 ` [PATCH 7/8] sta2x11-mfd : add defines for some sta2x11 sctl registers ciminaghi 2012-09-12 10:22 ` [PATCH 8/8] sta2x11-mfd : add myself to copyright ciminaghi 2012-09-16 21:25 ` [PATCH 0/8 RESEND] sta2x11-mfd patches Alessandro Rubini -- strict thread matches above, loose matches on Subject: below -- 2012-09-12 10:11 ciminaghi 2012-09-12 10:11 ` [PATCH 2/8] sta2x11-mfd : add sta2x11_mfd_get_regs_data() function ciminaghi
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).