Re: [irda-users] [PATCH] OMAP IrDA driver
Hi Felipe,
Sorry for delayed reply.
>> +
>> + void *rx_buf_dma_virt; /* Virtual address of RX DMA buffer */
>> + void *tx_buf_dma_virt; /* Virtual address of TX DMA buffer */
>
> should these two be void __iomem * ?
Agreed.
>
>> +
>> + struct device *dev;
>> + struct omap_irda_config *pdata;
>> +};
>> +
>> +static inline void uart_reg_out(int idx, u8 val)
>> +{
>> + omap_writeb(val, idx);
>
> __raw_writeb(), pass a reference to struct omap_irda here so you'd have
> access to a void __iomem *base (read below).
Agreed.
>> + */
>> +static void omap_irda_rx_dma_callback(int lch, u16 ch_status, void *data)
>> +{
>> + struct net_device *dev = data;
>> + struct omap_irda *omap_ir = netdev_priv(dev);
>> +
>> + printk(KERN_ERR "RX Transfer error or very big frame\n");
>
> you have a device pointer in omap_ir, use it and convert these to
> dev_dbg() calls.
Agree.
>
>> + if (omap_ir->pdata->transceiver_mode && machine_is_omap_h2()) {
>> + /* Is it select_irda on H2 ? */
>> + omap_writel(omap_readl(FUNC_MUX_CTRL_A) | 7,
>> + FUNC_MUX_CTRL_A);
>
> __raw_writel/__raw_readl.
Agree.
>
>> +static int omap_irda_probe(struct platform_device *pdev)
>> +{
>> + struct net_device *dev;
>> + struct omap_irda *omap_ir;
>> + struct omap_irda_config *pdata = pdev->dev.platform_data;
>> + unsigned int baudrate_mask;
>> + int err = 0;
>> + int irq = NO_IRQ;
>
> you should probably have a struct resource *res = platform_get_resource(pdev,
> IORESOURCE_MEM, 0);
>
> and use it to ioremap() the base address. Then you can stop using the
> omap_read[bsl]/omap_write[bsl] calls and switch over to standard
> __raw_read[bsl]/__raw_write[bsl] ones.
>
Right.
>> +
>> + dev = alloc_irdadev(sizeof(struct omap_irda));
>> + if (!dev)
>> + goto err_mem_1;
>> +
>> +
>
> one blank line only.
>
Ok.
>> +
>> + /* Any better way to avoid this? No. */
>> + if (machine_is_omap_h3() || machine_is_omap_h4())
>> + INIT_DELAYED_WORK(&omap_ir->pdata->gpio_expa, NULL);
>
> what does this mean ? what's that gpio_expa??
gpio_expa name here is misleading now. It is work name being scheduled
of sleeping gpio/i2c operations over pcf857x expander chip. No harm
otherwise.
>> +
>> +static int omap_irda_remove(struct platform_device *pdev)
>> +{
>> + struct net_device *dev = platform_get_drvdata(pdev);
>> +
>> + if (pdev) {
>> + unregister_netdev(dev);
>> + free_netdev(dev);
>> + }
>
> pdev is always true here, remove this if().
Ok.
>> +
>> +static char __initdata banner[] = KERN_INFO "OMAP IrDA driver
>> initializing\n";
>> +
>> +static int __init omap_irda_init(void)
>> +{
>> + printk(banner);
>
> you can remove this banner. No need for it.
Ok.
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [irda-users] [PATCH] OMAP IrDA driver
Hi Tony,
>
>> +
>> +/*
>> + * Set the IrDA communications speed.
>> + * Interrupt have to be disabled here.
>> + */
>> +static int omap_irda_startup(struct net_device *dev)
>> +{
>> + struct omap_irda *omap_ir = netdev_priv(dev);
>> +
>> + /* FIXME: use clk_* apis for UART3 clock*/
>> + /* Enable UART3 clock and set UART3 to IrDA mode */
>> + if (machine_is_omap_h2() || machine_is_omap_h3())
>> + omap_writel(omap_readl(MOD_CONF_CTRL_0) | (1 << 31) | (1 <<
>> 15),
>> + MOD_CONF_CTRL_0);
This looks like muxing on H3.
>> +
>> + /* Only for H2?
>> + */
>> + if (omap_ir->pdata->transceiver_mode && machine_is_omap_h2()) {
>> + /* Is it select_irda on H2 ? */
>> + omap_writel(omap_readl(FUNC_MUX_CTRL_A) | 7,
>> + FUNC_MUX_CTRL_A);
>> + omap_ir->pdata->transceiver_mode(omap_ir->dev, IR_SIRMODE);
>> + }
>> +
>
I can move this to board-h3.c file instead with platform data flag probably.
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [irda-users] [PATCH] OMAP IrDA driver
On Fri, Dec 05, 2008 at 03:18:10PM +0530, Trilok Soni wrote:
> +struct omap_irda {
> + unsigned char open;
> + int speed; /* Current IrDA speed */
> + int newspeed;
> +
> + struct net_device_stats stats;
> + struct irlap_cb *irlap;
> + struct qos_info qos;
> +
> + int rx_dma_channel;
> + int tx_dma_channel;
> +
> + dma_addr_t rx_buf_dma_phys; /* Physical address of RX DMA buffer */
> + dma_addr_t tx_buf_dma_phys; /* Physical address of TX DMA buffer */
> +
> + void *rx_buf_dma_virt; /* Virtual address of RX DMA buffer */
> + void *tx_buf_dma_virt; /* Virtual address of TX DMA buffer */
should these two be void __iomem * ?
> +
> + struct device *dev;
> + struct omap_irda_config *pdata;
> +};
> +
> +static inline void uart_reg_out(int idx, u8 val)
> +{
> + omap_writeb(val, idx);
__raw_writeb(), pass a reference to struct omap_irda here so you'd have
access to a void __iomem *base (read below).
> +}
> +
> +static inline u8 uart_reg_in(int idx)
> +{
> + u8 b = omap_readb(idx);
ditto.
> + return b;
> +}
> +
> +/* forward declarations */
> +static int omap_irda_set_speed(struct net_device *dev, int speed);
> +
> +static void omap_irda_start_rx_dma(struct omap_irda *omap_ir)
> +{
> + /* Configure DMA */
> + omap_set_dma_src_params(omap_ir->rx_dma_channel, 0x3, 0x0,
> + omap_ir->pdata->src_start,
> + 0, 0);
> +
> + omap_enable_dma_irq(omap_ir->rx_dma_channel, 0x01);
> +
> + omap_set_dma_dest_params(omap_ir->rx_dma_channel, 0x0, 0x1,
> + omap_ir->rx_buf_dma_phys,
> + 0, 0);
> +
> + omap_set_dma_transfer_params(omap_ir->rx_dma_channel, 0x0,
> + IRDA_SKB_MAX_MTU, 0x1,
> + 0x0, omap_ir->pdata->rx_trigger, 0);
> +
> + omap_start_dma(omap_ir->rx_dma_channel);
> +}
> +
> +static void omap_start_tx_dma(struct omap_irda *omap_ir, int size)
> +{
> + /* Configure DMA */
> + omap_set_dma_dest_params(omap_ir->tx_dma_channel, 0x03, 0x0,
> + omap_ir->pdata->dest_start, 0, 0);
> +
> + omap_enable_dma_irq(omap_ir->tx_dma_channel, 0x01);
> +
> + omap_set_dma_src_params(omap_ir->tx_dma_channel, 0x0, 0x1,
> + omap_ir->tx_buf_dma_phys,
> + 0, 0);
> +
> + omap_set_dma_transfer_params(omap_ir->tx_dma_channel, 0x0, size, 0x1,
> + 0x0, omap_ir->pdata->tx_trigger, 0);
> +
> + /* Start DMA */
> + omap_start_dma(omap_ir->tx_dma_channel);
> +}
> +
> +/* DMA RX callback - normally, we should not go here,
> + * it calls only if something is going wrong
> + */
> +static void omap_irda_rx_dma_callback(int lch, u16 ch_status, void *data)
> +{
> + struct net_device *dev = data;
> + struct omap_irda *omap_ir = netdev_priv(dev);
> +
> + printk(KERN_ERR "RX Transfer error or very big frame\n");
you have a device pointer in omap_ir, use it and convert these to
dev_dbg() calls.
ditto to all below.
> +static int omap_irda_startup(struct net_device *dev)
> +{
> + struct omap_irda *omap_ir = netdev_priv(dev);
> +
> + /* FIXME: use clk_* apis for UART3 clock*/
> + /* Enable UART3 clock and set UART3 to IrDA mode */
> + if (machine_is_omap_h2() || machine_is_omap_h3())
> + omap_writel(omap_readl(MOD_CONF_CTRL_0) | (1 << 31) | (1 << 15),
> + MOD_CONF_CTRL_0);
> +
> + /* Only for H2?
> + */
> + if (omap_ir->pdata->transceiver_mode && machine_is_omap_h2()) {
> + /* Is it select_irda on H2 ? */
> + omap_writel(omap_readl(FUNC_MUX_CTRL_A) | 7,
> + FUNC_MUX_CTRL_A);
__raw_writel/__raw_readl.
> +static int omap_irda_probe(struct platform_device *pdev)
> +{
> + struct net_device *dev;
> + struct omap_irda *omap_ir;
> + struct omap_irda_config *pdata = pdev->dev.platform_data;
> + unsigned int baudrate_mask;
> + int err = 0;
> + int irq = NO_IRQ;
you should probably have a struct resource *res = platform_get_resource(pdev,
IORESOURCE_MEM, 0);
and use it to ioremap() the base address. Then you can stop using the
omap_read[bsl]/omap_write[bsl] calls and switch over to standard
__raw_read[bsl]/__raw_write[bsl] ones.
> +
> + if (!pdata) {
> + printk(KERN_ERR "IrDA Platform data not supplied\n");
dev_dbg(&pdev->dev, "..");
> + return -ENOENT;
> + }
> +
> + if (!pdata->rx_channel || !pdata->tx_channel) {
> + printk(KERN_ERR "IrDA invalid rx/tx channel value\n");
dev_dbg(&pdev->dev, "..");
> + return -ENOENT;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq <= 0) {
> + printk(KERN_WARNING "no irq for IrDA\n");
dev_dbg(&pdev-
Re: [irda-users] [PATCH] OMAP IrDA driver
Hi,
Just one comment below.
* Trilok Soni <[EMAIL PROTECTED]> [081205 01:48]:
> Hi Samuel,
>
> On Fri, Dec 5, 2008 at 2:58 PM, <[EMAIL PROTECTED]> wrote:
> >
> > Hi,
> >
> > On Fri, 5 Dec 2008 12:04:29 +0530, "Trilok Soni" <[EMAIL PROTECTED]>
> > wrote:
> >> This time adding LKML too.
> > Could you please inline the patch so that we can have an easier review ?
>
> I don't have proper git-send-email integration with gmail, so I am
> going to copy/paste this patch here:
> diff --git a/drivers/net/irda/omap-ir.c b/drivers/net/irda/omap-ir.c
> new file mode 100644
> index 000..bf29585
> --- /dev/null
> +++ b/drivers/net/irda/omap-ir.c
> @@ -0,0 +1,893 @@
> +
> +/*
> + * Set the IrDA communications speed.
> + * Interrupt have to be disabled here.
> + */
> +static int omap_irda_startup(struct net_device *dev)
> +{
> + struct omap_irda *omap_ir = netdev_priv(dev);
> +
> + /* FIXME: use clk_* apis for UART3 clock*/
> + /* Enable UART3 clock and set UART3 to IrDA mode */
> + if (machine_is_omap_h2() || machine_is_omap_h3())
> + omap_writel(omap_readl(MOD_CONF_CTRL_0) | (1 << 31) | (1 << 15),
> + MOD_CONF_CTRL_0);
> +
> + /* Only for H2?
> + */
> + if (omap_ir->pdata->transceiver_mode && machine_is_omap_h2()) {
> + /* Is it select_irda on H2 ? */
> + omap_writel(omap_readl(FUNC_MUX_CTRL_A) | 7,
> + FUNC_MUX_CTRL_A);
> + omap_ir->pdata->transceiver_mode(omap_ir->dev, IR_SIRMODE);
> + }
> +
It would be best to get rid of the machine_is this or that code in the
drivers, and pass the necessary flags in the platform_data.
Regards,
Tony
--
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
Re: [irda-users] [PATCH] OMAP IrDA driver
Hi Samuel, On Fri, Dec 5, 2008 at 2:58 PM, <[EMAIL PROTECTED]> wrote: > > Hi, > > On Fri, 5 Dec 2008 12:04:29 +0530, "Trilok Soni" <[EMAIL PROTECTED]> > wrote: >> This time adding LKML too. > Could you please inline the patch so that we can have an easier review ? I don't have proper git-send-email integration with gmail, so I am going to copy/paste this patch here: >From e218cd5b2f29fa3ca342a5f4074a9e3cd3cacdad Mon Sep 17 00:00:00 2001 From: Trilok Soni <[EMAIL PROTECTED]> Date: Fri, 28 Nov 2008 16:49:44 +0530 Subject: [PATCH] OMAP IrDA driver Add Texas Instruments OMAP IrDA device driver, which supports SIR/MIR and FIR. Signed-off-by: Trilok Soni <[EMAIL PROTECTED]> --- drivers/net/irda/Kconfig |9 + drivers/net/irda/Makefile |1 + drivers/net/irda/omap-ir.c | 893 3 files changed, 903 insertions(+), 0 deletions(-) create mode 100644 drivers/net/irda/omap-ir.c diff --git a/drivers/net/irda/Kconfig b/drivers/net/irda/Kconfig index e631755..6af0c15 100644 --- a/drivers/net/irda/Kconfig +++ b/drivers/net/irda/Kconfig @@ -342,5 +342,14 @@ config MCS_FIR To compile it as a module, choose M here: the module will be called mcs7780. +config OMAP_IR + tristate "OMAP IrDA(SIR/MIR/FIR)" + depends on IRDA && ARCH_OMAP +help + Say Y here if you want to build support for the Texas Instruments + OMAP IrDA device driver, which supports SIR/MIR/FIR. This driver + relies on platform specific helper routines so available capabilities + may vary from one OMAP target to another. + endmenu diff --git a/drivers/net/irda/Makefile b/drivers/net/irda/Makefile index 5d20fde..72bbc2a 100644 --- a/drivers/net/irda/Makefile +++ b/drivers/net/irda/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_VIA_FIR) += via-ircc.o obj-$(CONFIG_PXA_FICP) += pxaficp_ir.o obj-$(CONFIG_MCS_FIR) += mcs7780.o obj-$(CONFIG_AU1000_FIR) += au1k_ir.o +obj-$(CONFIG_OMAP_IR) += omap-ir.o # SIR drivers obj-$(CONFIG_IRTTY_SIR)+= irtty-sir.o sir-dev.o # dongle drivers for SIR drivers diff --git a/drivers/net/irda/omap-ir.c b/drivers/net/irda/omap-ir.c new file mode 100644 index 000..bf29585 --- /dev/null +++ b/drivers/net/irda/omap-ir.c @@ -0,0 +1,893 @@ +/* + * BRIEF MODULE DESCRIPTION + * + * Infra-red driver for the OMAP1610-H2 and OMAP1710-H3 and H4 Platforms + * (SIR/MIR/FIR modes) + * (based on omap-sir.c) + * + * Copyright 2003 MontaVista Software Inc. + * Author: MontaVista Software, Inc. + *[EMAIL PROTECTED] + * + * Copyright 2004 Texas Instruments. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN + * NO EVENT SHALL THE AUTHOR BELIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * 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., + * 675 Mass Ave, Cambridge, MA 02139, USA. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include + +#define UART3_EFR_EN (1 << 4) +#define UART3_MCR_EN_TCR_TLR (1 << 6) + +#define UART3_LCR_WL_8 (3 << 0) +#define UART3_LCR_SP2 (1 << 2) +#define UART3_LCR_DIVEN(1 << 7) + +#define UART3_FCR_FIFO_EN (1 << 0) +#define UART3_FCR_FIFO_RX (1 << 1) +#define UART3_FCR_FIFO_TX (1 << 2) +#define UART3_FCR_FIFO_DMA1(1 << 3) +#define UART3_FCR_FIFO_TX_TRIG16 (1 << 4) +#define UART3_FCR_FIFO_RX_TRIG16 (1 << 6) +#define UART3_FCR_CONFIG (\ + UART3_FCR_FIFO_EN | UART3_FCR_FIFO_RX |\ + UART3_FCR_FIFO_TX | UART3_FCR_FIFO_DMA1 |\ + UART3_FCR_F
Re: [irda-users] [PATCH] OMAP IrDA driver
Hi, On Fri, 5 Dec 2008 12:04:29 +0530, "Trilok Soni" <[EMAIL PROTECTED]> wrote: > This time adding LKML too. Could you please inline the patch so that we can have an easier review ? Cheers, Samuel. -- 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
