From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> To: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Subject: Re: [PATCH V4] MXS: Implement DMA support into mxs-i2c Date: Sun, 16 Sep 2012 12:23:42 +0200 [thread overview] Message-ID: <20120916102342.GA933@pengutronix.de> (raw) In-Reply-To: <1345779871-7677-1-git-send-email-marex-ynQEQJNshbs@public.gmane.org> [-- Attachment #1: Type: text/plain, Size: 3893 bytes --] On Fri, Aug 24, 2012 at 05:44:31AM +0200, Marek Vasut wrote: > This patch implements DMA support into mxs-i2c. DMA transfers are now enabled > via DT. The DMA operation is enabled by default. > > Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> Unsurprisingly, I also couldn't get the mode switching between DMA and PIOQUEUE to work :( However, I did come up with an idea how to select DMA or PIOQUEUE per bus via devicetree. If we change the view from "I want PIOQUEUE" to "I don't want DMA", we could simply override the default DMA configuration in the devicetree with, e.g.: i2c0: i2c@80058000 { ... fsl,i2c-dma-channel = <>; }; So, no DMA channel set up will fall back to PIOQUEUE. This is more generic than a custom binding, because if one doesn't want DMA, don't configure it. That should also work with other devices which can fall back to something. I think this is worth trying, another advantage is that it doesn't expose something new to the user. So, there is no legacy support needed in case there will be a better way of configuration. So, here is a proof-of-concept patch which adds that to Marek's DMA implementation which I'd like to fold into it. I am about to prepare a proper series and will post it later today. Comments appreciated. Thanks, Wolfram diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index 1f58197..0198a43 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -35,10 +35,6 @@ #define DRIVER_NAME "mxs-i2c" -static bool use_pioqueue; -module_param(use_pioqueue, bool, 0); -MODULE_PARM_DESC(use_pioqueue, "Use PIOQUEUE mode for transfer instead of DMA"); - #define MXS_I2C_CTRL0 (0x00) #define MXS_I2C_CTRL0_SET (0x04) @@ -556,24 +552,13 @@ static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c) int ret; /* - * The MXS I2C DMA mode is prefered and enabled by default. - * The PIO mode is still supported, but should be used only - * for debuging purposes etc. - */ - i2c->dma_mode = !use_pioqueue; - if (!i2c->dma_mode) - dev_info(dev, "Using PIOQUEUE mode for I2C transfers!\n"); - - /* * TODO: This is a temporary solution and should be changed * to use generic DMA binding later when the helpers get in. */ ret = of_property_read_u32(node, "fsl,i2c-dma-channel", &i2c->dma_channel); - if (ret) { - dev_warn(dev, "Failed to get DMA channel, using PIOQUEUE!\n"); - i2c->dma_mode = 0; - } + if (ret) + i2c->dma_mode = false; ret = of_property_read_u32(node, "clock-frequency", &speed); if (ret) @@ -624,6 +609,13 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) if (err) return err; + /* + * The MXS I2C DMA mode is prefered and enabled by default. + * Ideally, we'd switch between PIOQUEUE and DMA depending on the + * message size, but neither Marek Vasut nor Wolfram Sang got this + * to work without resetting the controller completely :( + */ + i2c->dma_mode = true; i2c->dev = dev; i2c->speed = &mxs_i2c_95kHz_config; @@ -640,8 +632,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) i2c->dma_data.chan_irq = dmairq; i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c); if (!i2c->dmach) { - dev_err(dev, "Failed to request dma\n"); - return -ENODEV; + dev_err(dev, "Failed to request dma, falling back\n"); + i2c->dma_mode = false; } } @@ -668,6 +660,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) of_i2c_register_devices(adap); + dev_info(dev, "registered. DMA: %s\n", i2c->dma_mode ? "on" : "off"); + return 0; } -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: w.sang@pengutronix.de (Wolfram Sang) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH V4] MXS: Implement DMA support into mxs-i2c Date: Sun, 16 Sep 2012 12:23:42 +0200 [thread overview] Message-ID: <20120916102342.GA933@pengutronix.de> (raw) In-Reply-To: <1345779871-7677-1-git-send-email-marex@denx.de> On Fri, Aug 24, 2012 at 05:44:31AM +0200, Marek Vasut wrote: > This patch implements DMA support into mxs-i2c. DMA transfers are now enabled > via DT. The DMA operation is enabled by default. > > Signed-off-by: Marek Vasut <marex@denx.de> Unsurprisingly, I also couldn't get the mode switching between DMA and PIOQUEUE to work :( However, I did come up with an idea how to select DMA or PIOQUEUE per bus via devicetree. If we change the view from "I want PIOQUEUE" to "I don't want DMA", we could simply override the default DMA configuration in the devicetree with, e.g.: i2c0: i2c at 80058000 { ... fsl,i2c-dma-channel = <>; }; So, no DMA channel set up will fall back to PIOQUEUE. This is more generic than a custom binding, because if one doesn't want DMA, don't configure it. That should also work with other devices which can fall back to something. I think this is worth trying, another advantage is that it doesn't expose something new to the user. So, there is no legacy support needed in case there will be a better way of configuration. So, here is a proof-of-concept patch which adds that to Marek's DMA implementation which I'd like to fold into it. I am about to prepare a proper series and will post it later today. Comments appreciated. Thanks, Wolfram diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index 1f58197..0198a43 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -35,10 +35,6 @@ #define DRIVER_NAME "mxs-i2c" -static bool use_pioqueue; -module_param(use_pioqueue, bool, 0); -MODULE_PARM_DESC(use_pioqueue, "Use PIOQUEUE mode for transfer instead of DMA"); - #define MXS_I2C_CTRL0 (0x00) #define MXS_I2C_CTRL0_SET (0x04) @@ -556,24 +552,13 @@ static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c) int ret; /* - * The MXS I2C DMA mode is prefered and enabled by default. - * The PIO mode is still supported, but should be used only - * for debuging purposes etc. - */ - i2c->dma_mode = !use_pioqueue; - if (!i2c->dma_mode) - dev_info(dev, "Using PIOQUEUE mode for I2C transfers!\n"); - - /* * TODO: This is a temporary solution and should be changed * to use generic DMA binding later when the helpers get in. */ ret = of_property_read_u32(node, "fsl,i2c-dma-channel", &i2c->dma_channel); - if (ret) { - dev_warn(dev, "Failed to get DMA channel, using PIOQUEUE!\n"); - i2c->dma_mode = 0; - } + if (ret) + i2c->dma_mode = false; ret = of_property_read_u32(node, "clock-frequency", &speed); if (ret) @@ -624,6 +609,13 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) if (err) return err; + /* + * The MXS I2C DMA mode is prefered and enabled by default. + * Ideally, we'd switch between PIOQUEUE and DMA depending on the + * message size, but neither Marek Vasut nor Wolfram Sang got this + * to work without resetting the controller completely :( + */ + i2c->dma_mode = true; i2c->dev = dev; i2c->speed = &mxs_i2c_95kHz_config; @@ -640,8 +632,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) i2c->dma_data.chan_irq = dmairq; i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c); if (!i2c->dmach) { - dev_err(dev, "Failed to request dma\n"); - return -ENODEV; + dev_err(dev, "Failed to request dma, falling back\n"); + i2c->dma_mode = false; } } @@ -668,6 +660,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) of_i2c_register_devices(adap); + dev_info(dev, "registered. DMA: %s\n", i2c->dma_mode ? "on" : "off"); + return 0; } -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120916/589f15da/attachment.sig>
next prev parent reply other threads:[~2012-09-16 10:23 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-08-24 3:44 [PATCH V4] MXS: Implement DMA support into mxs-i2c Marek Vasut 2012-08-24 3:44 ` Marek Vasut [not found] ` <1345779871-7677-1-git-send-email-marex-ynQEQJNshbs@public.gmane.org> 2012-08-27 18:54 ` Fabio Estevam 2012-08-27 18:54 ` Fabio Estevam [not found] ` <CAOMZO5CSvP3nXNq4VLYALh+U9OZRnjtgXqTr7zsKdeHpUdJ4cQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-09-16 10:44 ` Wolfram Sang 2012-09-16 10:44 ` Wolfram Sang [not found] ` <20120916104436.GC933-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2012-09-16 14:44 ` Fabio Estevam 2012-09-16 14:44 ` Fabio Estevam 2012-09-16 10:23 ` Wolfram Sang [this message] 2012-09-16 10:23 ` Wolfram Sang [not found] ` <20120916102342.GA933-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2012-09-16 10:51 ` Marek Vasut 2012-09-16 10:51 ` Marek Vasut [not found] ` <201209161251.41895.marex-ynQEQJNshbs@public.gmane.org> 2012-09-16 11:12 ` Wolfram Sang 2012-09-16 11:12 ` Wolfram Sang [not found] ` <20120916111223.GD933-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2012-09-16 11:14 ` Marek Vasut 2012-09-16 11:14 ` Marek Vasut 2012-09-27 0:03 ` Marek Vasut 2012-09-27 0:03 ` Marek Vasut 2012-10-08 9:25 ` Wolfram Sang 2012-10-08 9:25 ` Wolfram Sang [not found] ` <20121008092530.GA22051-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2012-10-08 13:15 ` Marek Vasut 2012-10-08 13:15 ` Marek Vasut
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20120916102342.GA933@pengutronix.de \ --to=w.sang-bicnvbalz9megne8c9+irq@public.gmane.org \ --cc=fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=marex-ynQEQJNshbs@public.gmane.org \ --cc=shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.