OMAP1 IrDA driver doubts

OMAP1 IrDA driver doubts

[Komal Shah]

Hi All,

I am cleaning up Irda driver in-order to add H4 support for the same.

I want to know, what exactly this sequence of read/write gpio expander
is doing in 

A)
--------snip-------------
oma1610_irda_start(...)
...
#ifdef CONFIG_MACH_OMAP_H3
        if ((err = read_gpio_expa(&ioExpanderVal, 0x26))) {
                printk(KERN_ERR "Error reading from I/O EXPANDER \n");
                return err;
        }

        ioExpanderVal |= 0x40;  /* 'P6' Enable IRDA_TX and IRDA_RX */

        if ((err = write_gpio_expa(ioExpanderVal, 0x26))) {
                printk(KERN_ERR "Error writing to I/O EXPANDER \n");
                return err;
        }
#endif
--------snip-------------

Is it related to enabling power? As similar sequence is also there for
H4?

B) 
--------snip-------------
omap_irda_startup(...)
...
/* Enable UART3 clock and set UART3 to IrDA mode */
omap_writel(omap_readl(MOD_CONF_CTRL_0) | (1 << 31) | (1 << 15),
            MOD_CONF_CTRL_0);
--------snip-------------
As comment suggests that it is enabling UART3 clock, why don't we use
clk_use(...) api? And setting irda_mode from omap_irda_config function
like in usb->hmc_mode?

--------snip-------------
if (machine_is_omap_h2()) {
        omap_writel(omap_readl(FUNC_MUX_CTRL_A) | 7, FUNC_MUX_CTRL_A);
--------snip-------------

What is this muxing is doing? Is there any entry in mux table so that I
can use omap1_cfg_reg(...) function call?

C)
I will introduce platform_data->set_mode function, which will move 
set_h3_irda_mode() and setting firsel_gpio to h3 and h2 files resp. 

As I don't have much info about 16xx/17xx, I would appreciate any
clarfications on the above doubts. Thanx.

---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

RE: OMAP1 IrDA driver doubts

[Komal Shah]

--- "G Kondaiah, Manjunath" <xgkondai@ti.com> wrote:

> 
> For H3 - Only bit 31 is valid for enabling UART clock. Setting bit 15
> has no effect.  You can remove this and use clock api's.  I am not
> sure
> about H2. 
> 
> drivers/net/irda/omap-ir.c -
> 
> it is better if UART defines are moved to arch specific header file
> 
> include/asm-arm/arch-omap/irda.h  - copyright header is missing.

Thanx for the review. I agree with your comments. I will make necessary
changes in the next release of patch.

---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

RE: OMAP1 IrDA driver doubts

[G Kondaiah, Manjunath]

B)
>>>--------snip-------------
>>>omap_irda_startup(...)
>>>...
>>>/* Enable UART3 clock and set UART3 to IrDA mode */
>>>omap_writel(omap_readl(MOD_CONF_CTRL_0) | (1 << 31) | (1 << 15),
>>>            MOD_CONF_CTRL_0);
>>>--------snip-------------
>>>As comment suggests that it is enabling UART3 clock, why don't we use
>>>clk_use(...) api? And setting irda_mode from omap_irda_config
function like in >>>usb->hmc_mode?

For H3 - Only bit 31 is valid for enabling UART clock. Setting bit 15
has no effect.  You can remove this and use clock api's.  I am not sure
about H2. 

drivers/net/irda/omap-ir.c -

it is better if UART defines are moved to arch specific header file

include/asm-arm/arch-omap/irda.h  - copyright header is missing.

Regards,
Manjunath

-----Original Message-----
From: Komal Shah [mailto:komal_shah802003@yahoo.com] 
Sent: Wednesday, December 14, 2005 6:08 AM
To: G Kondaiah, Manjunath; linux-omap-open-source@linux.omap.com
Subject: RE: OMAP1 IrDA driver doubts

--- "G Kondaiah, Manjunath" <xgkondai@ti.com> wrote:

> Hi Komal,
> 
> A) is required since the same lines are shared between IrDA and aGPS
> modules. There is an I/O Expander to select these lines on H3/H4
> using
> I2C.

Thanx. I have taken it as set_power function for a while in the recent
patch posted on the list for 24xx irda support, but I will try to
rename it something different in the next patch.

Could you please comment on the B) and C) also? _or_ It would be gr8,
if you give me some patch review comments :)

---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

RE: OMAP1 IrDA driver doubts

[Komal Shah]

--- "G Kondaiah, Manjunath" <xgkondai@ti.com> wrote:

> Hi Komal,
> 
> A) is required since the same lines are shared between IrDA and aGPS
> modules. There is an I/O Expander to select these lines on H3/H4
> using
> I2C.

Thanx. I have taken it as set_power function for a while in the recent
patch posted on the list for 24xx irda support, but I will try to
rename it something different in the next patch.

Could you please comment on the B) and C) also? _or_ It would be gr8,
if you give me some patch review comments :)

---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

RE: OMAP1 IrDA driver doubts

[G Kondaiah, Manjunath]

Hi Komal,

A) is required since the same lines are shared between IrDA and aGPS
modules. There is an I/O Expander to select these lines on H3/H4 using
I2C.

Regards,
Manjunath

-----Original Message-----
From: linux-omap-open-source-bounces+xgkondai=ti.com@linux.omap.com
[mailto:linux-omap-open-source-bounces+xgkondai=ti.com@linux.omap.com]
On Behalf Of Komal Shah
Sent: Tuesday, December 13, 2005 11:20 AM
To: linux-omap-open-source@linux.omap.com
Subject: OMAP1 IrDA driver doubts

Hi All,

I am cleaning up Irda driver in-order to add H4 support for the same.

I want to know, what exactly this sequence of read/write gpio expander
is doing in 

A)
--------snip-------------
oma1610_irda_start(...)
...
#ifdef CONFIG_MACH_OMAP_H3
        if ((err = read_gpio_expa(&ioExpanderVal, 0x26))) {
                printk(KERN_ERR "Error reading from I/O EXPANDER \n");
                return err;
        }

        ioExpanderVal |= 0x40;  /* 'P6' Enable IRDA_TX and IRDA_RX */

        if ((err = write_gpio_expa(ioExpanderVal, 0x26))) {
                printk(KERN_ERR "Error writing to I/O EXPANDER \n");
                return err;
        }
#endif
--------snip-------------

Is it related to enabling power? As similar sequence is also there for
H4?

B) 
--------snip-------------
omap_irda_startup(...)
...
/* Enable UART3 clock and set UART3 to IrDA mode */
omap_writel(omap_readl(MOD_CONF_CTRL_0) | (1 << 31) | (1 << 15),
            MOD_CONF_CTRL_0);
--------snip-------------
As comment suggests that it is enabling UART3 clock, why don't we use
clk_use(...) api? And setting irda_mode from omap_irda_config function
like in usb->hmc_mode?

--------snip-------------
if (machine_is_omap_h2()) {
        omap_writel(omap_readl(FUNC_MUX_CTRL_A) | 7, FUNC_MUX_CTRL_A);
--------snip-------------

What is this muxing is doing? Is there any entry in mux table so that I
can use omap1_cfg_reg(...) function call?

C)
I will introduce platform_data->set_mode function, which will move 
set_h3_irda_mode() and setting firsel_gpio to h3 and h2 files resp. 

As I don't have much info about 16xx/17xx, I would appreciate any
clarfications on the above doubts. Thanx.

---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
_______________________________________________
Linux-omap-open-source mailing list
Linux-omap-open-source@linux.omap.com
http://linux.omap.com/mailman/listinfo/linux-omap-open-source