RE: [PATCH] serial: Add OMAP high-speed UART driver.
> -Original Message- > From: Shilimkar, Santosh > Sent: Tuesday, March 02, 2010 8:43 PM > To: G, Manjunath Kondaiah; S, Venkatraman; Tony Lindgren; > Raja, Govindraj; Greg KH; linux-ser...@vger.kernel.org; > linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org; > Kevin Hilman; Olof Johansson > Subject: RE: [PATCH] serial: Add OMAP high-speed UART driver. > > > -Original Message- > > From: G, Manjunath Kondaiah > > Sent: Tuesday, March 02, 2010 8:34 PM > > To: Shilimkar, Santosh; S, Venkatraman; Tony Lindgren; > Raja, Govindraj; Greg KH; linux- > > ser...@vger.kernel.org; linux-omap@vger.kernel.org; > linux-ker...@vger.kernel.org; Kevin Hilman; Olof > > Johansson > > Subject: RE: [PATCH] serial: Add OMAP high-speed UART driver. > > > > > > Santosh, > > > > > -Original Message- > > > From: Shilimkar, Santosh > > > Sent: Tuesday, March 02, 2010 7:34 PM > > > To: G, Manjunath Kondaiah; S, Venkatraman; Tony Lindgren; > > > Raja, Govindraj; Greg KH; linux-ser...@vger.kernel.org; > > > linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org; > > > Kevin Hilman; Olof Johansson > > > Subject: RE: [PATCH] serial: Add OMAP high-speed UART driver. > > > > > > > > > > > > -- > > > > > CDAC is a shadow register used for monitoring the DMA channel. > > > > > I think it would be a lot > > > > > simpler if omap_start_dma() always resets CDAC to 0, and the > > > > > UART driver > > > > > just not set it explicitly. > > > > > > > > This seems to be better option than exposing CDAC read/write API > > > > to other drivers since user need to write '0' before > > > starting any DMA > > > > transfer which can be be done in omap_start_dma(). > > > > > > > > I am wondering how other drivers are using DMA transfer > > > API's without > > > > resetting CDAC to zero. > > > > > > > It's needed only if some one is interested in that count. > > > UART seems to > > > using this counter where as other driver don't. > > > > > > Why do you think drivers need to know about counter value ? > > > > Reading of non zero value(after reset to zero and enabling > dma channel) > > from CDAC register indicates that, DMA transfer has started > and user can > > rely on DMA4_CCENi and DMA4_CCFNi element and frame counters. > > > > If the CDAC value is zero even after starting DMA channel, indicates > > error. > Not necessary. The DMA can still wait for the hw sync signal > and the CDAC > can remain 0 if the hw sync in not received. This will be any > way returned > by DMA error ( SYNC lost) > > This register was mainly suppose to be used for debug purposes. As per TRM, CDAC can also be used for monitoring synchroized data transfer apart from hw sync signal monitoring. Anyway, we have are aligned to set this register to zero in omap_start_dma() which will over ride this patch. Regards, Manjunath -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
> -Original Message- > From: G, Manjunath Kondaiah > Sent: Tuesday, March 02, 2010 8:34 PM > To: Shilimkar, Santosh; S, Venkatraman; Tony Lindgren; Raja, Govindraj; Greg > KH; linux- > ser...@vger.kernel.org; linux-omap@vger.kernel.org; > linux-ker...@vger.kernel.org; Kevin Hilman; Olof > Johansson > Subject: RE: [PATCH] serial: Add OMAP high-speed UART driver. > > > Santosh, > > > -Original Message- > > From: Shilimkar, Santosh > > Sent: Tuesday, March 02, 2010 7:34 PM > > To: G, Manjunath Kondaiah; S, Venkatraman; Tony Lindgren; > > Raja, Govindraj; Greg KH; linux-ser...@vger.kernel.org; > > linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org; > > Kevin Hilman; Olof Johansson > > Subject: RE: [PATCH] serial: Add OMAP high-speed UART driver. > > > > > > > > > -- > > > > CDAC is a shadow register used for monitoring the DMA channel. > > > > I think it would be a lot > > > > simpler if omap_start_dma() always resets CDAC to 0, and the > > > > UART driver > > > > just not set it explicitly. > > > > > > This seems to be better option than exposing CDAC read/write API > > > to other drivers since user need to write '0' before > > starting any DMA > > > transfer which can be be done in omap_start_dma(). > > > > > > I am wondering how other drivers are using DMA transfer > > API's without > > > resetting CDAC to zero. > > > > > It's needed only if some one is interested in that count. > > UART seems to > > using this counter where as other driver don't. > > > > Why do you think drivers need to know about counter value ? > > Reading of non zero value(after reset to zero and enabling dma channel) > from CDAC register indicates that, DMA transfer has started and user can > rely on DMA4_CCENi and DMA4_CCFNi element and frame counters. > > If the CDAC value is zero even after starting DMA channel, indicates > error. Not necessary. The DMA can still wait for the hw sync signal and the CDAC can remain 0 if the hw sync in not received. This will be any way returned by DMA error ( SYNC lost) This register was mainly suppose to be used for debug purposes. Regards, Santosh -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
Santosh, > -Original Message- > From: Shilimkar, Santosh > Sent: Tuesday, March 02, 2010 7:34 PM > To: G, Manjunath Kondaiah; S, Venkatraman; Tony Lindgren; > Raja, Govindraj; Greg KH; linux-ser...@vger.kernel.org; > linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org; > Kevin Hilman; Olof Johansson > Subject: RE: [PATCH] serial: Add OMAP high-speed UART driver. > > > > > > -- > > > CDAC is a shadow register used for monitoring the DMA channel. > > > I think it would be a lot > > > simpler if omap_start_dma() always resets CDAC to 0, and the > > > UART driver > > > just not set it explicitly. > > > > This seems to be better option than exposing CDAC read/write API > > to other drivers since user need to write '0' before > starting any DMA > > transfer which can be be done in omap_start_dma(). > > > > I am wondering how other drivers are using DMA transfer > API's without > > resetting CDAC to zero. > > > It's needed only if some one is interested in that count. > UART seems to > using this counter where as other driver don't. > > Why do you think drivers need to know about counter value ? Reading of non zero value(after reset to zero and enabling dma channel) from CDAC register indicates that, DMA transfer has started and user can rely on DMA4_CCENi and DMA4_CCFNi element and frame counters. If the CDAC value is zero even after starting DMA channel, indicates error. -Manjunath -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
> > > -- > > CDAC is a shadow register used for monitoring the DMA channel. > > I think it would be a lot > > simpler if omap_start_dma() always resets CDAC to 0, and the > > UART driver > > just not set it explicitly. > > This seems to be better option than exposing CDAC read/write API > to other drivers since user need to write '0' before starting any DMA > transfer which can be be done in omap_start_dma(). > > I am wondering how other drivers are using DMA transfer API's without > resetting CDAC to zero. > It's needed only if some one is interested in that count. UART seems to using this counter where as other driver don't. Why do you think drivers need to know about counter value ? Regards, Santosh -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
> -Original Message- > From: svenk...@gmail.com [mailto:svenk...@gmail.com] On > Behalf Of Venkatraman S > Sent: Tuesday, March 02, 2010 7:16 PM > To: Tony Lindgren; G, Manjunath Kondaiah; Raja, Govindraj; > Greg KH; linux-ser...@vger.kernel.org; > linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org; > Kevin Hilman; Olof Johansson > Subject: Re: [PATCH] serial: Add OMAP high-speed UART driver. > > On Tue, Mar 2, 2010 at 12:22 PM, Tony Lindgren > wrote: > > * G, Manjunath Kondaiah [100301 22:24]: > >> > >> > >> > >> > > > > + up->uart_dma.prev_rx_dma_pos = > >> > up->uart_dma.rx_buf_dma_phys; > >> > > > > + if (cpu_is_omap44xx()) > >> > > > > + omap_writel(0, OMAP44XX_DMA4_BASE > >> > > > > + + > >> > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); > >> > > > > + else > >> > > > > + omap_writel(0, OMAP34XX_DMA4_BASE > >> > > > > + + > >> > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); > >> > > > > >> > > > NAK. Please don't use omap_read/write for for new > code. And do not > >> > > > tinker with the omap hardware registers directly in > the driver. > >> > > > > >> > > > This needs to be done properly in > >> > arch/arm/plat-omap/dma.c instead. > >> > > > >> > > Thanks for the suggestion. > >> > > > >> > > Currently, dma_read/dma_write are #define's in dma.c which > >> > cannot be > >> > > accessed outside dma.c. I don't see any API's in dma.c for > >> > setting required > >> > > value for this register? > >> > > >> > Hmm isn't this the same as omap_get_dma_dst_pos(int > lch)? If you're > >> > trying do something that's not in dma.c, we can add a > new function > >> > for it. > >> > >> The omap_get_dma_dst_pos(int lch) is for read operation in > CDAC register. > >> But, We need to write required value into CDAC register. > For this, I propose: > >> > >> omap_set_dma_dst_pos(int lch, int value) which does not > exist in current dma > >> driver. > > > > OK, it that's needed. > > > > Tony > > -- > CDAC is a shadow register used for monitoring the DMA channel. > I think it would be a lot > simpler if omap_start_dma() always resets CDAC to 0, and the > UART driver > just not set it explicitly. This seems to be better option than exposing CDAC read/write API to other drivers since user need to write '0' before starting any DMA transfer which can be be done in omap_start_dma(). I am wondering how other drivers are using DMA transfer API's without resetting CDAC to zero. -Manjunath -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
On Tue, Mar 2, 2010 at 12:22 PM, Tony Lindgren wrote: > * G, Manjunath Kondaiah [100301 22:24]: >> >> >> >> > > > > + up->uart_dma.prev_rx_dma_pos = >> > up->uart_dma.rx_buf_dma_phys; >> > > > > + if (cpu_is_omap44xx()) >> > > > > + omap_writel(0, OMAP44XX_DMA4_BASE >> > > > > + + >> > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); >> > > > > + else >> > > > > + omap_writel(0, OMAP34XX_DMA4_BASE >> > > > > + + >> > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); >> > > > >> > > > NAK. Please don't use omap_read/write for for new code. And do not >> > > > tinker with the omap hardware registers directly in the driver. >> > > > >> > > > This needs to be done properly in >> > arch/arm/plat-omap/dma.c instead. >> > > >> > > Thanks for the suggestion. >> > > >> > > Currently, dma_read/dma_write are #define's in dma.c which >> > cannot be >> > > accessed outside dma.c. I don't see any API's in dma.c for >> > setting required >> > > value for this register? >> > >> > Hmm isn't this the same as omap_get_dma_dst_pos(int lch)? If you're >> > trying do something that's not in dma.c, we can add a new function >> > for it. >> >> The omap_get_dma_dst_pos(int lch) is for read operation in CDAC register. >> But, We need to write required value into CDAC register. For this, I propose: >> >> omap_set_dma_dst_pos(int lch, int value) which does not exist in current dma >> driver. > > OK, it that's needed. > > Tony > -- CDAC is a shadow register used for monitoring the DMA channel. I think it would be a lot simpler if omap_start_dma() always resets CDAC to 0, and the UART driver just not set it explicitly. Simple wrapper APIs doing get / set on individual DMA registers is as difficult to understand as omap_read / omap_write, IMHO. Best regards, Venkat. -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
* G, Manjunath Kondaiah [100301 22:24]: > > > > > > > > + up->uart_dma.prev_rx_dma_pos = > > up->uart_dma.rx_buf_dma_phys; > > > > > + if (cpu_is_omap44xx()) > > > > > + omap_writel(0, OMAP44XX_DMA4_BASE > > > > > + + > > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); > > > > > + else > > > > > + omap_writel(0, OMAP34XX_DMA4_BASE > > > > > + + > > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); > > > > > > > > NAK. Please don't use omap_read/write for for new code. And do not > > > > tinker with the omap hardware registers directly in the driver. > > > > > > > > This needs to be done properly in > > arch/arm/plat-omap/dma.c instead. > > > > > > Thanks for the suggestion. > > > > > > Currently, dma_read/dma_write are #define's in dma.c which > > cannot be > > > accessed outside dma.c. I don't see any API's in dma.c for > > setting required > > > value for this register? > > > > Hmm isn't this the same as omap_get_dma_dst_pos(int lch)? If you're > > trying do something that's not in dma.c, we can add a new function > > for it. > > The omap_get_dma_dst_pos(int lch) is for read operation in CDAC register. > But, We need to write required value into CDAC register. For this, I propose: > > omap_set_dma_dst_pos(int lch, int value) which does not exist in current dma > driver. OK, it that's needed. Tony -- 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: [PATCH] serial: Add OMAP high-speed UART driver.
> > > > + up->uart_dma.prev_rx_dma_pos = > up->uart_dma.rx_buf_dma_phys; > > > > + if (cpu_is_omap44xx()) > > > > + omap_writel(0, OMAP44XX_DMA4_BASE > > > > + + > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); > > > > + else > > > > + omap_writel(0, OMAP34XX_DMA4_BASE > > > > + + > OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel)); > > > > > > NAK. Please don't use omap_read/write for for new code. And do not > > > tinker with the omap hardware registers directly in the driver. > > > > > > This needs to be done properly in > arch/arm/plat-omap/dma.c instead. > > > > Thanks for the suggestion. > > > > Currently, dma_read/dma_write are #define's in dma.c which > cannot be > > accessed outside dma.c. I don't see any API's in dma.c for > setting required > > value for this register? > > Hmm isn't this the same as omap_get_dma_dst_pos(int lch)? If you're > trying do something that's not in dma.c, we can add a new function > for it. The omap_get_dma_dst_pos(int lch) is for read operation in CDAC register. But, We need to write required value into CDAC register. For this, I propose: omap_set_dma_dst_pos(int lch, int value) which does not exist in current dma driver. -Manjunath-- 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: [PATCH] serial: Add OMAP high-speed UART driver.
* G, Manjunath Kondaiah [100301 21:48]:
> Tony,
>
> > -Original Message-
> > From: linux-omap-ow...@vger.kernel.org
> > [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Tony Lindgren
> > Sent: Tuesday, March 02, 2010 12:16 AM
> > To: Raja, Govindraj
> > Cc: Greg KH; linux-ser...@vger.kernel.org;
> > linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org;
> > Kevin Hilman; Olof Johansson
> > Subject: Re: [PATCH] serial: Add OMAP high-speed UART driver.
> >
> > * Govindraj.R [100301 06:40]:
> > > --- /dev/null
> > > +++ b/drivers/serial/omap-serial.c
> > > +
> > > +static void serial_omap_stop_tx(struct uart_port *port)
> > > +{
> > > + struct uart_omap_port *up = (struct uart_omap_port *)port;
> > > +
> > > + if (up->use_dma &&
> > > + up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
> > > + /*
> > > + * Check if dma is still active . If yes do nothing,
> >
> > Looks like an extra space here before the period above.
> >
> > ...
> >
> > > +static int serial_omap_start_rxdma(struct uart_omap_port *up)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (up->uart_dma.rx_dma_channel == -1) {
> > > + ret = omap_request_dma(up->uart_dma.uart_dma_rx,
> > > + "UART Rx DMA",
> > > + (void *)uart_rx_dma_callback, up,
> > > + &(up->uart_dma.rx_dma_channel));
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + omap_set_dma_src_params(up->uart_dma.rx_dma_channel, 0,
> > > + OMAP_DMA_AMODE_CONSTANT,
> > > + up->uart_dma.uart_base, 0, 0);
> > > + omap_set_dma_dest_params(up->uart_dma.rx_dma_channel, 0,
> > > + OMAP_DMA_AMODE_POST_INC,
> > > + up->uart_dma.rx_buf_dma_phys, 0, 0);
> > > +
> > omap_set_dma_transfer_params(up->uart_dma.rx_dma_channel,
> > > + OMAP_DMA_DATA_TYPE_S8,
> > > + up->uart_dma.rx_buf_size, 1,
> > > + OMAP_DMA_SYNC_ELEMENT,
> > > + up->uart_dma.uart_dma_rx, 0);
> > > + }
> > > + up->uart_dma.prev_rx_dma_pos = up->uart_dma.rx_buf_dma_phys;
> > > + if (cpu_is_omap44xx())
> > > + omap_writel(0, OMAP44XX_DMA4_BASE
> > > + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
> > > + else
> > > + omap_writel(0, OMAP34XX_DMA4_BASE
> > > + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
> >
> > NAK. Please don't use omap_read/write for for new code. And do not
> > tinker with the omap hardware registers directly in the driver.
> >
> > This needs to be done properly in arch/arm/plat-omap/dma.c instead.
>
> Thanks for the suggestion.
>
> Currently, dma_read/dma_write are #define's in dma.c which cannot be
> accessed outside dma.c. I don't see any API's in dma.c for setting required
> value for this register?
Hmm isn't this the same as omap_get_dma_dst_pos(int lch)? If you're
trying do something that's not in dma.c, we can add a new function
for it.
> Can we move dma_read/dma_write to header file so that it can be global or
> Is there alternate way to perform this operation with existing dma driver?
No thanks, that again leads all drivers messing with the DMA registers
directly and can easily lead to errors.
Regards,
Tony
--
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: [PATCH] serial: Add OMAP high-speed UART driver.
Tony,
> -Original Message-
> From: linux-omap-ow...@vger.kernel.org
> [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Tony Lindgren
> Sent: Tuesday, March 02, 2010 12:16 AM
> To: Raja, Govindraj
> Cc: Greg KH; linux-ser...@vger.kernel.org;
> linux-omap@vger.kernel.org; linux-ker...@vger.kernel.org;
> Kevin Hilman; Olof Johansson
> Subject: Re: [PATCH] serial: Add OMAP high-speed UART driver.
>
> * Govindraj.R [100301 06:40]:
> > --- /dev/null
> > +++ b/drivers/serial/omap-serial.c
> > +
> > +static void serial_omap_stop_tx(struct uart_port *port)
> > +{
> > + struct uart_omap_port *up = (struct uart_omap_port *)port;
> > +
> > + if (up->use_dma &&
> > + up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
> > + /*
> > +* Check if dma is still active . If yes do nothing,
>
> Looks like an extra space here before the period above.
>
> ...
>
> > +static int serial_omap_start_rxdma(struct uart_omap_port *up)
> > +{
> > + int ret = 0;
> > +
> > + if (up->uart_dma.rx_dma_channel == -1) {
> > + ret = omap_request_dma(up->uart_dma.uart_dma_rx,
> > + "UART Rx DMA",
> > + (void *)uart_rx_dma_callback, up,
> > + &(up->uart_dma.rx_dma_channel));
> > + if (ret < 0)
> > + return ret;
> > +
> > + omap_set_dma_src_params(up->uart_dma.rx_dma_channel, 0,
> > + OMAP_DMA_AMODE_CONSTANT,
> > + up->uart_dma.uart_base, 0, 0);
> > + omap_set_dma_dest_params(up->uart_dma.rx_dma_channel, 0,
> > + OMAP_DMA_AMODE_POST_INC,
> > + up->uart_dma.rx_buf_dma_phys, 0, 0);
> > +
> omap_set_dma_transfer_params(up->uart_dma.rx_dma_channel,
> > + OMAP_DMA_DATA_TYPE_S8,
> > + up->uart_dma.rx_buf_size, 1,
> > + OMAP_DMA_SYNC_ELEMENT,
> > + up->uart_dma.uart_dma_rx, 0);
> > + }
> > + up->uart_dma.prev_rx_dma_pos = up->uart_dma.rx_buf_dma_phys;
> > + if (cpu_is_omap44xx())
> > + omap_writel(0, OMAP44XX_DMA4_BASE
> > + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
> > + else
> > + omap_writel(0, OMAP34XX_DMA4_BASE
> > + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
>
> NAK. Please don't use omap_read/write for for new code. And do not
> tinker with the omap hardware registers directly in the driver.
>
> This needs to be done properly in arch/arm/plat-omap/dma.c instead.
Thanks for the suggestion.
Currently, dma_read/dma_write are #define's in dma.c which cannot be
accessed outside dma.c. I don't see any API's in dma.c for setting required
value for this register?
Can we move dma_read/dma_write to header file so that it can be global or
Is there alternate way to perform this operation with existing dma driver?
-Manjunath--
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: [PATCH] serial: Add OMAP high-speed UART driver.
* Govindraj.R [100301 06:40]:
> --- /dev/null
> +++ b/drivers/serial/omap-serial.c
> +
> +static void serial_omap_stop_tx(struct uart_port *port)
> +{
> + struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> + if (up->use_dma &&
> + up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
> + /*
> + * Check if dma is still active . If yes do nothing,
Looks like an extra space here before the period above.
...
> +static int serial_omap_start_rxdma(struct uart_omap_port *up)
> +{
> + int ret = 0;
> +
> + if (up->uart_dma.rx_dma_channel == -1) {
> + ret = omap_request_dma(up->uart_dma.uart_dma_rx,
> + "UART Rx DMA",
> + (void *)uart_rx_dma_callback, up,
> + &(up->uart_dma.rx_dma_channel));
> + if (ret < 0)
> + return ret;
> +
> + omap_set_dma_src_params(up->uart_dma.rx_dma_channel, 0,
> + OMAP_DMA_AMODE_CONSTANT,
> + up->uart_dma.uart_base, 0, 0);
> + omap_set_dma_dest_params(up->uart_dma.rx_dma_channel, 0,
> + OMAP_DMA_AMODE_POST_INC,
> + up->uart_dma.rx_buf_dma_phys, 0, 0);
> + omap_set_dma_transfer_params(up->uart_dma.rx_dma_channel,
> + OMAP_DMA_DATA_TYPE_S8,
> + up->uart_dma.rx_buf_size, 1,
> + OMAP_DMA_SYNC_ELEMENT,
> + up->uart_dma.uart_dma_rx, 0);
> + }
> + up->uart_dma.prev_rx_dma_pos = up->uart_dma.rx_buf_dma_phys;
> + if (cpu_is_omap44xx())
> + omap_writel(0, OMAP44XX_DMA4_BASE
> + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
> + else
> + omap_writel(0, OMAP34XX_DMA4_BASE
> + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
NAK. Please don't use omap_read/write for for new code. And do not
tinker with the omap hardware registers directly in the driver.
This needs to be done properly in arch/arm/plat-omap/dma.c instead.
We really don't want drivers tinkering with the omap hardware
registers directly except for the driver block registers.
Regards,
Tony
--
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: [PATCH] serial: Add OMAP high-speed UART driver.
> + 213 = /dev/ttyO0 OMAP serial port 0 > + 214 = /dev/ttyO1 OMAP serial port 1 > + 215 = /dev/ttyO2 OMAP serial port 2 > + 216 = /dev/ttyO3 OMAP serial port 3 I think it might be best to allocate a dynamic tty major nowdays Other problem is that I don't see where you hold the port lock over all uses of port->tty. That is an assumption of the serial layer and done in the original 8250 driver correctly. Rest looks good and I agree that OMAP is different enough from 8250 that it makes sense not to further clutter the 8250 driver. -- 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
