Hi,
On Thu, Dec 17, 2009 at 08:40:32PM +0100, ext Candelaria Villareal, Jorge wrote:
diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index 61952aa..b96e9d8 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -6,6 +6,9 @@ config SND_OMAP_SOC_MCBSP
tristate
select OMAP_MCBSP
+config SND_OMAP_SOC_MCPDM
+ tristate
look at how SND_OMAP_SOC_N810 is done, can't you follow that ? put some
description ad help ?
diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c
new file mode 100644
index 0000000..e162dd1
--- /dev/null
+++ b/sound/soc/omap/mcpdm.c
@@ -0,0 +1,505 @@
+/*
+ * mcpdm.c -- McPDM interface driver
+ *
+ * Author: Jorge Eduardo Candelaria <[email protected]>
no copyright ? How about:
Copyright (C) 2009 - Texas Instruments, Inc.
+ * 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 published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/wait.h>
+#include <linux/interrupt.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+
+#include "mcpdm.h"
is this used by any other file ? If not, it shouldn't be necessary.
+static struct omap_mcpdm *mcpdm;
allocate on probe() and make it the private_data of the device structure
with platform_set_drvdata(pdev, mcpdm);
+static void omap_mcpdm_write(u16 reg, u32 val)
I'd rather:
static inline void omap_mcpdm_write(struct omap_mcpdm *mcpdm,
u16 reg, u32 val)
+{
+ __raw_writel(val, mcpdm->io_base + reg);
+}
+
+static int omap_mcpdm_read(u16 reg)
ditto.
+{
+ return __raw_readl(mcpdm->io_base + reg);
+}
+
+void omap_mcpdm_reg_dump(void)
+{
+ dev_dbg(mcpdm->dev, "***********************\n");
+ dev_dbg(mcpdm->dev, "IRQSTATUS_RAW: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_IRQSTATUS_RAW));
+ dev_dbg(mcpdm->dev, "IRQSTATUS: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_IRQSTATUS));
+ dev_dbg(mcpdm->dev, "IRQENABLE_SET: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_IRQENABLE_SET));
+ dev_dbg(mcpdm->dev, "IRQENABLE_CLR: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_IRQENABLE_CLR));
+ dev_dbg(mcpdm->dev, "IRQWAKE_EN: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_IRQWAKE_EN));
+ dev_dbg(mcpdm->dev, "DMAENABLE_SET: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_DMAENABLE_SET));
+ dev_dbg(mcpdm->dev, "DMAENABLE_CLR: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_DMAENABLE_CLR));
+ dev_dbg(mcpdm->dev, "DMAWAKEEN: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_DMAWAKEEN));
+ dev_dbg(mcpdm->dev, "CTRL: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_CTRL));
+ dev_dbg(mcpdm->dev, "DN_DATA: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_DN_DATA));
+ dev_dbg(mcpdm->dev, "UP_DATA: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_UP_DATA));
+ dev_dbg(mcpdm->dev, "FIFO_CTRL_DN: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_FIFO_CTRL_DN));
+ dev_dbg(mcpdm->dev, "FIFO_CTRL_UP: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_FIFO_CTRL_UP));
+ dev_dbg(mcpdm->dev, "DN_OFFSET: 0x%04x\n",
+ omap_mcpdm_read(MCPDM_DN_OFFSET));
+ dev_dbg(mcpdm->dev, "***********************\n");
+}
+EXPORT_SYMBOL(omap_mcpdm_reg_dump);
I'd rather use debugfs for such stuff.
+EXPORT_SYMBOL(omap_mcpdm_set_offset);
+EXPORT_SYMBOL(omap_mcpdm_reset);
+EXPORT_SYMBOL(omap_mcpdm_start);
+EXPORT_SYMBOL(omap_mcpdm_stop);
+EXPORT_SYMBOL(omap_mcpdm_set_uplink);
+EXPORT_SYMBOL(omap_mcpdm_set_downlink);
+EXPORT_SYMBOL(omap_mcpdm_clr_uplink);
+EXPORT_SYMBOL(omap_mcpdm_clr_downlink);
+EXPORT_SYMBOL(omap_mcpdm_request);
+EXPORT_SYMBOL(omap_mcpdm_free);
way too many exported symbols, no ? Doesn't ALSA API have proper place
for this kind of stuff ? I'd need ALSA experts to reply to that but it
does smell funny...
+static irqreturn_t omap_mcpdm_irq_handler(int irq, void *dev_id)
+{
+ struct omap_mcpdm *mcpdm_irq = dev_id;
+ int irq_status;
+
+ irq_status = omap_mcpdm_read(MCPDM_IRQSTATUS);
+
+ /* Acknowledge irq event */
+ omap_mcpdm_write(MCPDM_IRQSTATUS, irq_status);
+
+ switch (irq_status) {
it's better if you:
if (irq_status & DN_IRQ_FULL) {
...
}
if (irq_status & DN_IRQ_EMPTY) {
...
}
if (irq_status & DN_IRQ) {
...
}
+int omap_mcpdm_request(void)
+{
+ int ret;
+
+ clk_enable(mcpdm->clk);
+
+ spin_lock(&mcpdm->lock);
+
+ if (!mcpdm->free) {
+ dev_err(mcpdm->dev, "McPDM interface is in use\n");
+ spin_unlock(&mcpdm->lock);
+ return -EBUSY;
+ }
+ mcpdm->free = 0;
+
+ spin_unlock(&mcpdm->lock);
+
+ /* Disable lines while request is ongoing */
+ omap_mcpdm_write(MCPDM_CTRL, 0x00);
+
+ ret = request_irq(mcpdm->irq, omap_mcpdm_irq_handler,
+ 0, "McPDM", (void *)mcpdm);
+ if (ret) {
+ dev_err(mcpdm->dev, "Request for McPDM IRQ failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(omap_mcpdm_request);
How about moving this to probe() ??
+void omap_mcpdm_free(void)
+{
+ clk_disable(mcpdm->clk);
+
+ spin_lock(&mcpdm->lock);
+ if (mcpdm->free) {
+ dev_err(mcpdm->dev, "McPDM interface is already free\n");
+ spin_unlock(&mcpdm->lock);
+ return;
+ }
+ mcpdm->free = 1;
+ spin_unlock(&mcpdm->lock);
+
+ free_irq(mcpdm->irq, (void *)mcpdm);
+}
+EXPORT_SYMBOL(omap_mcpdm_free);
move to remove()
+static int __devinit omap_mcpdm_probe(struct platform_device *pdev)
+{
+ struct omap_mcpdm_platform_data *pdata = pdev->dev.platform_data;
+ int ret = 0;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "McPDM device initialized without "
+ "platform data\n");
try not to break the string, it makes grep much more difficult. You can
also make a smaller string like: "no platform data" or something...
+ ret = -EINVAL;
+ goto exit;
+ }
+ dev_dbg(&pdev->dev, "Initializing OMAP McPDM driver \n");
not needed this message.
+ mcpdm = kzalloc(sizeof(struct omap_mcpdm), GFP_KERNEL);
+ if (!mcpdm) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ spin_lock_init(&mcpdm->lock);
+ mcpdm->free = 1;
+ mcpdm->phys_base = pdata->phys_base;
this is passed via struct resource.
+ mcpdm->io_base = ioremap(mcpdm->phys_base, SZ_4K);
then this would be something like:
static int __devinit omap_mcpdm_probe(struct platform_device *pdev)
{
...
struct resource *res;
...
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res == NULL) {
dev_err(&pdev->dev, "no resource\n");
goto err1;
}
mcpdm->io_base = ioremap(res->start, resource_size(res));
if (mcpdm->io_base == NULL) {
dev_err(&pdev->dev, "ioremap failed\n");
goto err2;
}
...
+ if (!mcpdm->io_base) {
+ ret = -ENOMEM;
+ goto err_ioremap;
+ }
+
+ mcpdm->irq = pdata->irq;
+
+ /* FIXME: Enable this ones correct clk nodes available */
+ if (!cpu_is_omap44xx()) {
if you don't use the code now, don't add it.
+ mcpdm->clk = clk_get(&pdev->dev, "fclk");
+ if (IS_ERR(mcpdm->clk)) {
+ ret = PTR_ERR(mcpdm->clk);
+ dev_err(&pdev->dev, "unable to get fclk: %d\n", ret);
+ goto err_clk;
+ }
+ }
+
+ mcpdm->pdata = pdata;
pdata is supposed to be __initdata, no point in saving it.
+ mcpdm->dev = &pdev->dev;
+ platform_set_drvdata(pdev, mcpdm);
+
+ return 0;
+
+err_clk:
+ iounmap(mcpdm->io_base);
+err_ioremap:
+ kfree(mcpdm);
+exit:
+ return ret;
+}
+
+static int __devexit omap_mcpdm_remove(struct platform_device *pdev)
+{
+ struct omap_mcpdm *mcpdm_ptr = platform_get_drvdata(pdev);
+
+ platform_set_drvdata(pdev, NULL);
+
+ if (mcpdm_ptr) {
if this pointer is NULL, the you deserve to oops, get rid of the branch.
+ clk_disable(mcpdm_ptr->clk);
+ clk_put(mcpdm_ptr->clk);
+
+ iounmap(mcpdm_ptr->io_base);
+
+ mcpdm_ptr->clk = NULL;
+ mcpdm_ptr->free = 0;
+ mcpdm_ptr->dev = NULL;
where's your kfree(mcpdm_ptr) ???
+ printk(KERN_INFO "McPDM driver removed \n");
get rid of this.
+static struct platform_driver omap_mcpdm_driver = {
+ .probe = omap_mcpdm_probe,
+ .remove = omap_mcpdm_remove,
.remove = __devexit_p(omap_mcpdm_remove),
+ .driver = {
+ .name = "omap-mcpdm",
+ },
+};
+
+static struct platform_device *omap_mcpdm_device;
+
+static struct omap_mcpdm_platform_data mcpdm_pdata = {
+ .phys_base = OMAP44XX_MCPDM_BASE,
+ .irq = INT_44XX_MCPDM_IRQ,
+};
this should be passed by arch code, no ??
+static int __init omap_mcpdm_init(void)
+{
+ int ret;
+ struct platform_device *device;
+
+ device = platform_device_alloc("omap-mcpdm", -1);
+ if (!device) {
+ printk(KERN_ERR "McPDM platform device allocation failed\n");
+ return -ENOMEM;
+ }
+ device->dev.platform_data = &mcpdm_pdata;
although n810 does the same, I don't like it. I think the board file
should register the platform_device
+ omap_mcpdm_device = device;
+ (void) platform_device_add(omap_mcpdm_device);
what ? so even if you don't have a platform_device you register the
driver ??
+ ret = platform_driver_register(&omap_mcpdm_driver);
+ if (ret)
+ goto error;
+ return 0;
+
+error:
+ printk(KERN_ERR "OMAP McPDM initialization error\n");
no printk(), simply return err.
+ return ret;
+}
+arch_initcall(omap_mcpdm_init);
I wonder if arch_initcall() is really necessary...
diff --git a/sound/soc/omap/mcpdm.h b/sound/soc/omap/mcpdm.h
new file mode 100644
index 0000000..c953e95
--- /dev/null
+++ b/sound/soc/omap/mcpdm.h
@@ -0,0 +1,156 @@
+/*
+ * mcpdm.h -- Defines for McPDM driver
+ *
+ * Author: Jorge Eduardo Candelaria <[email protected]>
+ *
+ * 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 published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#define OMAP44XX_MCPDM_BASE 0x40132000
+#define OMAP44XX_MCPDM_L3_BASE 0x49032000
not here... probably in some <plat/cpu.h> or similar...
+/* McPDM registers */
+
+#define MCPDM_REVISION 0x00
+#define MCPDM_SYSCONFIG 0x10
+#define MCPDM_IRQSTATUS_RAW 0x24
+#define MCPDM_IRQSTATUS 0x28
+#define MCPDM_IRQENABLE_SET 0x2C
+#define MCPDM_IRQENABLE_CLR 0x30
+#define MCPDM_IRQWAKE_EN 0x34
+#define MCPDM_DMAENABLE_SET 0x38
+#define MCPDM_DMAENABLE_CLR 0x3C
+#define MCPDM_DMAWAKEEN 0x40
+#define MCPDM_CTRL 0x44
+#define MCPDM_DN_DATA 0x48
+#define MCPDM_UP_DATA 0x4C
+#define MCPDM_FIFO_CTRL_DN 0x50
+#define MCPDM_FIFO_CTRL_UP 0x54
+#define MCPDM_DN_OFFSET 0x58
+
+/*
+ * MCPDM_IRQ bit fields
+ * IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR
+ */
+
+#define DN_IRQ (1 << 0)
+#define DN_IRQ_EMTPY (1 << 1)
+#define DN_IRQ_ALMST_EMPTY (1 << 2)
+#define DN_IRQ_FULL (1 << 3)
+#define UP_IRQ (1 << 8)
+#define UP_IRQ_EMPTY (1 << 9)
+#define UP_IRQ_ALMST_FULL (1 << 10)
+#define UP_IRQ_FULL (1 << 11)
+
+#define DOWNLINK_IRQ_MASK 0x00F
+#define UPLINK_IRQ_MASK 0xF00
prepend with MCPDM_
+void omap_mcpdm_reg_dump(void);
+void omap_mcpdm_reset(int links, int reset);
+void omap_mcpdm_start(int stream);
+void omap_mcpdm_stop(int stream);
+int omap_mcpdm_set_uplink(struct omap_mcpdm_link *uplink);
+int omap_mcpdm_set_downlink(struct omap_mcpdm_link *downlink);
+int omap_mcpdm_clr_uplink(struct omap_mcpdm_link *uplink);
+int omap_mcpdm_clr_downlink(struct omap_mcpdm_link *downlink);
+int omap_mcpdm_request(void);
+void omap_mcpdm_free(void);
+int omap_mcpdm_set_offset(int offset1, int offset2);
these shouldn't be necessary but at least the extern is missing.
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
