* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Paul Walmsley @ 2012-10-11 18:28 UTC (permalink / raw)
To: Poddar\, Sourav, Felipe Balbi
Cc: Kevin Hilman, Russell King - ARM Linux, gregkh, tony,
linux-kernel, santosh.shilimkar, linux-serial, linux-omap,
linux-arm-kernel, alan
In-Reply-To: <87ipas2y4h.fsf@deeprootsystems.com>
Hi Sourav, Felipe,
any progress on fixing the N800 problem? Would be good to keep it booting
since we use it as our primary 2420 test platform.
- Paul
^ permalink raw reply
* Re: [PATCH] serial: clps711x: reworked driver version
From: Greg Kroah-Hartman @ 2012-10-11 18:34 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: linux-serial, Alan Cox
In-Reply-To: <1349970689-24198-1-git-send-email-shc_work@mail.ru>
On Thu, Oct 11, 2012 at 07:51:29PM +0400, Alexander Shiyan wrote:
> This patch presents reworked version of CLPS711X serial driver.
> The changes from the old version:
> - Driver converted to platform_device.
> - Using CPU clock subsystem for getting base UART speed, since CPU can run
> on different speeds.
> - Remove console_initcall and make console dynamically. Earler messages in
> this case can be retrieved by using "earlyprintk" kernel option.
> - Using resource-managed functions (devm_xx).
> - Make all variables dynamically (reduce BSS).
> - Cleanup code & comments.
That's a lot of different things, all at once, making it very hard to
review.
Can you please break this up into the individual patches of what you are
doing above, one patch per thing, as is needed for Linux kernel patches?
That way it is much easier to accept and review.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] serial: clps711x: reworked driver version
From: Alexander Shiyan @ 2012-10-11 18:57 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-serial, Alan Cox
In-Reply-To: <20121011183406.GC32470@kroah.com>
On Fri, 12 Oct 2012 03:34:06 +0900
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Thu, Oct 11, 2012 at 07:51:29PM +0400, Alexander Shiyan wrote:
> > This patch presents reworked version of CLPS711X serial driver.
> > The changes from the old version:
> > - Driver converted to platform_device.
> > - Using CPU clock subsystem for getting base UART speed, since CPU can run
> > on different speeds.
> > - Remove console_initcall and make console dynamically. Earler messages in
> > this case can be retrieved by using "earlyprintk" kernel option.
> > - Using resource-managed functions (devm_xx).
> > - Make all variables dynamically (reduce BSS).
> > - Cleanup code & comments.
>
> That's a lot of different things, all at once, making it very hard to
> review.
> Can you please break this up into the individual patches of what you are
> doing above, one patch per thing, as is needed for Linux kernel patches?
> That way it is much easier to accept and review.
It's too hard to do, since each modification depends on the other...
Maybe make a patch through renaming, for example clps711x_uart? Or you
still insist to split it into separate patches?
--
Alexander Shiyan <shc_work@mail.ru>
^ permalink raw reply
* Re: [PATCH] serial: clps711x: reworked driver version
From: Greg Kroah-Hartman @ 2012-10-11 19:20 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: linux-serial, Alan Cox
In-Reply-To: <20121011225703.3f930bf1.shc_work@mail.ru>
On Thu, Oct 11, 2012 at 10:57:03PM +0400, Alexander Shiyan wrote:
> On Fri, 12 Oct 2012 03:34:06 +0900
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>
> > On Thu, Oct 11, 2012 at 07:51:29PM +0400, Alexander Shiyan wrote:
> > > This patch presents reworked version of CLPS711X serial driver.
> > > The changes from the old version:
> > > - Driver converted to platform_device.
> > > - Using CPU clock subsystem for getting base UART speed, since CPU can run
> > > on different speeds.
> > > - Remove console_initcall and make console dynamically. Earler messages in
> > > this case can be retrieved by using "earlyprintk" kernel option.
> > > - Using resource-managed functions (devm_xx).
> > > - Make all variables dynamically (reduce BSS).
> > > - Cleanup code & comments.
> >
> > That's a lot of different things, all at once, making it very hard to
> > review.
> > Can you please break this up into the individual patches of what you are
> > doing above, one patch per thing, as is needed for Linux kernel patches?
> > That way it is much easier to accept and review.
>
> It's too hard to do, since each modification depends on the other...
That's why you do one patch after the other, a series of patches, all
depending on the previous ones.
> Maybe make a patch through renaming, for example clps711x_uart? Or you
> still insist to split it into separate patches?
Linux kernel development is all about individual patches, each one only
doing one thing. We've been doing this for 20+ years now, it's not
anything "new" :)
thanks,
greg k-h
^ permalink raw reply
* 8250_early for big-endian
From: Noam Camus @ 2012-10-12 13:05 UTC (permalink / raw)
To: linux-serial@vger.kernel.org
Hello
I am using 8250 console and wish to use the 8250_early as well.
I encounter endianess problem while using this driver for mmio32.
The problem is since my serial peripheral like my cpu is in BIG endian.
Driver through serial_in/serial_out uses readl/writel from asm-generic/io.h.
These macros assume peripheral is always in LITTLE endian.
Therefore all data needs a swab.
In 8250 driver I can register replacement for default serial_in/serial_out and workaround the problem.
In 8250_early I had no choice but to change code with additional swab to workaround the problem.
I am looking for proper solution so it can be added to upstream.
I got several ideas if interest arise.
Waiting for your comments
Noam Camus
^ permalink raw reply
* Re: [RFC 00/24] OMAP serial driver flow control fixes, and preparation for DMA engine conversion
From: Grazvydas Ignotas @ 2012-10-12 14:51 UTC (permalink / raw)
To: Sourav
Cc: Russell King - ARM Linux, Kevin Hilman, Tony Lindgren,
Greg Kroah-Hartman, linux-serial, linux-omap, linux-arm-kernel,
Alan Cox
In-Reply-To: <50769D8C.8030401@ti.com>
On Thu, Oct 11, 2012 at 1:21 PM, Sourav <sourav.poddar@ti.com> wrote:
> If you have any pointers on how to test hardware flow control, I will like
> to do that on my beagle board.
If you can find some board with BT chip connected to UART, that would
be a good test as they usually have flow control lines connected and
can run at high bitrates. I think some EVM or Zoom boards have BT
chips.
--
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Sourav @ 2012-10-12 16:24 UTC (permalink / raw)
To: Paul Walmsley
Cc: Felipe Balbi, Kevin Hilman, Russell King - ARM Linux, gregkh,
tony, linux-kernel, santosh.shilimkar, linux-serial, linux-omap,
linux-arm-kernel, alan
In-Reply-To: <alpine.DEB.2.00.1210111748010.5736@utopia.booyaka.com>
Hi Paul,
There are
On Thursday 11 October 2012 11:58 PM, Paul Walmsley wrote:
> Hi Sourav, Felipe,
>
> any progress on fixing the N800 problem? Would be good to keep it booting
> since we use it as our primary 2420 test platform.
>
>
> - Paul
The patch sent inlined below might help us to get rid of the serial init
issue.
Unfortunately, I dont have a N800 board with me to test it and will require
your help to do so.
-----------
From: Sourav Poddar <sourav.poddar@ti.com>
Date: Wed, 1 Aug 2012 15:44:12 +0530
Subject: [RFT/PATCH] serial: omap: Fix N800 serial init issue.
This patch might solve the N800 serial init issue.
This patch will also give pointers if there is any mux settings issue
with N800 OR
a mismatch between the initial harware state, runtime PM state and omap
hwmod state.
I don't have a N800 schematics to check about the mux settings getting used.
The observation on beagle board XM with this patch on different boards
looks flaky,
so your feedback on beagle board will also be very helpful.
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
drivers/tty/serial/omap-serial.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 6ede6fd..3fbc7f7 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
platform_device *pdev)
INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
platform_set_drvdata(pdev, up);
+ pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_autosuspend_delay(&pdev->dev,
--
1.7.1
^ permalink raw reply related
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Kevin Hilman @ 2012-10-12 16:35 UTC (permalink / raw)
To: Sourav
Cc: Paul Walmsley, Felipe Balbi, Russell King - ARM Linux, gregkh,
tony, linux-kernel, santosh.shilimkar, linux-serial, linux-omap,
linux-arm-kernel, alan
In-Reply-To: <50784458.9080806@ti.com>
Sourav <sourav.poddar@ti.com> writes:
> Hi Paul,
>
> There are
> On Thursday 11 October 2012 11:58 PM, Paul Walmsley wrote:
>> Hi Sourav, Felipe,
>>
>> any progress on fixing the N800 problem? Would be good to keep it booting
>> since we use it as our primary 2420 test platform.
>>
>>
>> - Paul
> The patch sent inlined below might help us to get rid of the serial
> init issue.
> Unfortunately, I dont have a N800 board with me to test it and will require
> your help to do so.
> -----------
> From: Sourav Poddar <sourav.poddar@ti.com>
> Date: Wed, 1 Aug 2012 15:44:12 +0530
> Subject: [RFT/PATCH] serial: omap: Fix N800 serial init issue.
>
>
> This patch might solve the N800 serial init issue.
>
> This patch will also give pointers if there is any mux settings issue
> with N800 OR
> a mismatch between the initial harware state, runtime PM state and
> omap hwmod state.
> I don't have a N800 schematics to check about the mux settings getting used.
>
> The observation on beagle board XM with this patch on different boards
> looks flaky,
> so your feedback on beagle board will also be very helpful.
>
> Cc: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> drivers/tty/serial/omap-serial.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c
> b/drivers/tty/serial/omap-serial.c
> index 6ede6fd..3fbc7f7 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
> platform_device *pdev)
> INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
>
> platform_set_drvdata(pdev, up);
> + pm_runtime_set_active(&pdev->dev);
NAK.
This will obviously break platforms where the UARTs are not active
before driver loads.
Kevin
> pm_runtime_enable(&pdev->dev);
> pm_runtime_use_autosuspend(&pdev->dev);
> pm_runtime_set_autosuspend_delay(&pdev->dev,
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Russell King - ARM Linux @ 2012-10-12 16:42 UTC (permalink / raw)
To: Kevin Hilman
Cc: Sourav, Paul Walmsley, Felipe Balbi, gregkh, tony, linux-kernel,
santosh.shilimkar, linux-serial, linux-omap, linux-arm-kernel,
alan
In-Reply-To: <877gqv7imt.fsf@deeprootsystems.com>
On Fri, Oct 12, 2012 at 09:35:54AM -0700, Kevin Hilman wrote:
> Sourav <sourav.poddar@ti.com> writes:
> > diff --git a/drivers/tty/serial/omap-serial.c
> > b/drivers/tty/serial/omap-serial.c
> > index 6ede6fd..3fbc7f7 100644
> > --- a/drivers/tty/serial/omap-serial.c
> > +++ b/drivers/tty/serial/omap-serial.c
> > @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
> > platform_device *pdev)
> > INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
> >
> > platform_set_drvdata(pdev, up);
> > + pm_runtime_set_active(&pdev->dev);
>
> NAK.
>
> This will obviously break platforms where the UARTs are not active
> before driver loads.
I thought I had proposed a solution for this issue, which was this
sequence:
omap_device_enable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
Yes, I can understand people not liking the omap_device_enable()
there, but I also notice that the email suggesting that never got a
reply either - not even a "I tried this and it doesn't work" or "it
does work".
As such, it seems this issue isn't making any progress as we had
already established that merely doing a "pm_runtime_set_active()"
before "pm_runtime_enable()" was going to break other platforms.
^ permalink raw reply
* Re: 8250_early for big-endian
From: Alan Cox @ 2012-10-12 16:51 UTC (permalink / raw)
To: Noam Camus; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <264C179F799EF24AB26D5319053335E80CC30499A2@ezexch.ezchip.com>
> I am using 8250 console and wish to use the 8250_early as well.
>
> I encounter endianess problem while using this driver for mmio32.
>
> The problem is since my serial peripheral like my cpu is in BIG endian.
>
> Driver through serial_in/serial_out uses readl/writel from asm-generic/io.h.
>
> These macros assume peripheral is always in LITTLE endian.
>
> Therefore all data needs a swab.
>
> In 8250 driver I can register replacement for default serial_in/serial_out and workaround the problem.
> In 8250_early I had no choice but to change code with additional swab to workaround the problem.
>
> I am looking for proper solution so it can be added to upstream.
> I got several ideas if interest arise.
Given they are all byte registers so how can they need swapping ? They are
merely 3 further bytes offset.
Alan
^ permalink raw reply
* RE: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Poddar, Sourav @ 2012-10-12 17:29 UTC (permalink / raw)
To: Russell King - ARM Linux, Kevin Hilman
Cc: Paul Walmsley, Balbi, Felipe, gregkh@linuxfoundation.org,
tony@atomide.com, linux-kernel@vger.kernel.org,
Shilimkar, Santosh, linux-serial@vger.kernel.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
alan@linux.intel.com
In-Reply-To: <20121012164202.GQ28061@n2100.arm.linux.org.uk>
Hi Russell,
________________________________________
From: Russell King - ARM Linux [linux@arm.linux.org.uk]
Sent: Friday, October 12, 2012 10:12 PM
To: Kevin Hilman
Cc: Poddar, Sourav; Paul Walmsley; Balbi, Felipe; gregkh@linuxfoundation.org; tony@atomide.com; linux-kernel@vger.kernel.org; Shilimkar, Santosh; linux-serial@vger.kernel.org; linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; alan@linux.intel.com
Subject: Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
On Fri, Oct 12, 2012 at 09:35:54AM -0700, Kevin Hilman wrote:
> Sourav <sourav.poddar@ti.com> writes:
> > diff --git a/drivers/tty/serial/omap-serial.c
> > b/drivers/tty/serial/omap-serial.c
> > index 6ede6fd..3fbc7f7 100644
> > --- a/drivers/tty/serial/omap-serial.c
> > +++ b/drivers/tty/serial/omap-serial.c
> > @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
> > platform_device *pdev)
> > INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
> >
> > platform_set_drvdata(pdev, up);
> > + pm_runtime_set_active(&pdev->dev);
>
> NAK.
>
> This will obviously break platforms where the UARTs are not active
> before driver loads.
I thought I had proposed a solution for this issue, which was this
sequence:
omap_device_enable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
Yes, I can understand people not liking the omap_device_enable()
there, but I also notice that the email suggesting that never got a
reply either - not even a "I tried this and it doesn't work" or "it
does work".
Sorry for the late reply on this. I tried this sequence and it worked perfectly fine on
panda and beagle.
As such, it seems this issue isn't making any progress as we had
already established that merely doing a "pm_runtime_set_active()"
before "pm_runtime_enable()" was going to break other platforms.
I was trying to analyse your explanations on this and since omap_device_enable is not generally
recommended, I was trying to see if anything else can be done to get around this.
I send this patch for N800 testing so as to see how it behaves. (We are suspecting that there might be
mux setting issue also with N800).
~Sourav
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Kevin Hilman @ 2012-10-12 17:59 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Sourav, Paul Walmsley, Felipe Balbi, gregkh, tony, linux-kernel,
santosh.shilimkar, linux-serial, linux-omap, linux-arm-kernel,
alan
In-Reply-To: <20121012164202.GQ28061@n2100.arm.linux.org.uk>
Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Fri, Oct 12, 2012 at 09:35:54AM -0700, Kevin Hilman wrote:
>> Sourav <sourav.poddar@ti.com> writes:
>> > diff --git a/drivers/tty/serial/omap-serial.c
>> > b/drivers/tty/serial/omap-serial.c
>> > index 6ede6fd..3fbc7f7 100644
>> > --- a/drivers/tty/serial/omap-serial.c
>> > +++ b/drivers/tty/serial/omap-serial.c
>> > @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
>> > platform_device *pdev)
>> > INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
>> >
>> > platform_set_drvdata(pdev, up);
>> > + pm_runtime_set_active(&pdev->dev);
>>
>> NAK.
>>
>> This will obviously break platforms where the UARTs are not active
>> before driver loads.
>
> I thought I had proposed a solution for this issue, which was this
> sequence:
>
> omap_device_enable(dev);
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> Yes, I can understand people not liking the omap_device_enable()
> there, but I also notice that the email suggesting that never got a
> reply either - not even a "I tried this and it doesn't work" or "it
> does work".
Yes, that solution would work (though I didn't actually try it.)
However, we can't use omap_device_enable() in the driver. We're trying
to clean all the drivers of OMAP-specific APIs. That being said,
something similar could be done in the device/board init code to ensure
the UART HW is in the state that the driver is expecting it, but again,
that would just mask the real problem which is that a (re)init of the
console UART on 2420/n800 is causing output to disappear.
As I detailed in an earlier response, I still think it's the fact that
the pinmux is not setup correctly for the console UART pins in the board
file, so when the UART is initialized, its mux settings are changed from
the bootloader defaults, causing output to disappear.
> As such, it seems this issue isn't making any progress as we had
> already established that merely doing a "pm_runtime_set_active()"
> before "pm_runtime_enable()" was going to break other platforms.
Agreed.
Kevin
^ permalink raw reply
* RE: 8250_early for big-endian
From: Noam Camus @ 2012-10-12 18:41 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <20121012175150.4b081343@pyramind.ukuu.org.uk>
________________________________________
>From: Alan Cox [alan@lxorguk.ukuu.org.uk]
>Sent: Friday, October 12, 2012 6:51 PM
>To: Noam Camus
>Cc: linux-serial@vger.kernel.org
>Subject: Re: 8250_early for big-endian
>
>> I am using 8250 console and wish to use the 8250_early as well.
>>
>> I encounter endianess problem while using this driver for mmio32.
.>>
>> The problem is since my serial peripheral like my cpu is in BIG endian.
>>
>> Driver through serial_in/serial_out uses readl/writel from asm-generic/io.h.
>>
>> These macros assume peripheral is always in LITTLE endian.
>>
>> Therefore all data needs a swab.
>>
>> In 8250 driver I can register replacement for default serial_in/serial_out and workaround the problem.
>> In 8250_early I had no choice but to change code with additional swab to workaround the problem.
>>
>> I am looking for proper solution so it can be added to upstream.
>> I got several ideas if interest arise.
>
>Given they are all byte registers so how can they need swapping ? They are
>merely 3 further bytes offset.
>
>Alan
Of course swapping is a waste of cpu cycles.
Workaround can be 3 further bytes offset.
Thanks
Back to my problem.
How can 8250_early be enhanced to support big endian registers.
Is it configuation?
Maybe command line flag?
both? other?
Noam
^ permalink raw reply
* Re: 8250_early for big-endian
From: Alan Cox @ 2012-10-12 18:51 UTC (permalink / raw)
To: Noam Camus; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <264C179F799EF24AB26D5319053335E80CC30499A4@ezexch.ezchip.com>
> Of course swapping is a waste of cpu cycles.
> Workaround can be 3 further bytes offset.
> Thanks
>
> Back to my problem.
> How can 8250_early be enhanced to support big endian registers.
They are 8bit registers - I don't understand why you are asking the
question ?
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Russell King - ARM Linux @ 2012-10-12 18:49 UTC (permalink / raw)
To: Poddar, Sourav
Cc: Kevin Hilman, Paul Walmsley, Balbi, Felipe,
gregkh@linuxfoundation.org, tony@atomide.com,
linux-kernel@vger.kernel.org, Shilimkar, Santosh,
linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, alan@linux.intel.com
In-Reply-To: <FEB1293C1A07484EA4E9721B32981131CC2362@DBDE01.ent.ti.com>
On Fri, Oct 12, 2012 at 05:29:55PM +0000, Poddar, Sourav wrote:
> Hi Russell,
> ________________________________________
> From: Russell King - ARM Linux [linux@arm.linux.org.uk]
> Sent: Friday, October 12, 2012 10:12 PM
> To: Kevin Hilman
> Cc: Poddar, Sourav; Paul Walmsley; Balbi, Felipe; gregkh@linuxfoundation.org; tony@atomide.com; linux-kernel@vger.kernel.org; Shilimkar, Santosh; linux-serial@vger.kernel.org; linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; alan@linux.intel.com
> Subject: Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
>
> On Fri, Oct 12, 2012 at 09:35:54AM -0700, Kevin Hilman wrote:
> > Sourav <sourav.poddar@ti.com> writes:
> > > diff --git a/drivers/tty/serial/omap-serial.c
> > > b/drivers/tty/serial/omap-serial.c
> > > index 6ede6fd..3fbc7f7 100644
> > > --- a/drivers/tty/serial/omap-serial.c
> > > +++ b/drivers/tty/serial/omap-serial.c
> > > @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
> > > platform_device *pdev)
> > > INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
> > >
> > > platform_set_drvdata(pdev, up);
> > > + pm_runtime_set_active(&pdev->dev);
> >
> > NAK.
> >
> > This will obviously break platforms where the UARTs are not active
> > before driver loads.
>
> I thought I had proposed a solution for this issue, which was this
> sequence:
>
> omap_device_enable(dev);
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> Yes, I can understand people not liking the omap_device_enable()
> there, but I also notice that the email suggesting that never got a
> reply either - not even a "I tried this and it doesn't work" or "it
> does work".
>
> Sorry for the late reply on this. I tried this sequence and it worked perfectly fine on
> panda and beagle.
>
> As such, it seems this issue isn't making any progress as we had
> already established that merely doing a "pm_runtime_set_active()"
> before "pm_runtime_enable()" was going to break other platforms.
>
> I was trying to analyse your explanations on this and since
> omap_device_enable is not generally recommended, I was trying to see
> if anything else can be done to get around this.
Right, so what you need is explanation about why I believe the above
sequence to be necessary.
What is happening is that we're starting from a device in unknown state.
We don't know whether it is enabled or disabled. We don't know the
state of the clocks or the power domain.
PM runtime state is initialized at device creation in the "device is
suspended" state. If we merely enable PM runtime from that state, we
are telling the PM runtime subsystem that the device is _indeed_
suspended (disabled) at boot time.
So, that causes the first pm_runtime_get() call to resume the device.
Due to the OMAP runtime PM hooks at the bus layer, this causes
_od_runtime_resume() to be called.
_od_runtime_resume() does two things. It calls omap_device_enable()
to ensure that the device is woken up (such as, ensuring that the
power domain is on, and turning on the clocks etc.) It then goes on
to call the device PM layers to call the driver specific runtime PM
resume hook.
So, in summary, what this sequence does is:
pm_runtime_enable(&pdev->dev);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_autosuspend_delay(&pdev->dev,
omap_up_info->autosuspend_timeout);
pm_runtime_irq_safe(&pdev->dev);
pm_runtime_get_sync(&pdev->dev);
is, at the last call, it calls:
_od_runtime_resume()
omap_device_enable()
serial_omap_runtime_resume()
Your original patch at the head of this thread says that the driver
specific runtime resume call causes a problem for N800 - because the
device is already enabled and setup.
Okay, so the initial device state does not match the runtime PM state.
So, what happens if we _do_ make it match your state - as required by
the runtime PM documentation - by adding a call before the sequence:
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_autosuspend_delay(&pdev->dev,
omap_up_info->autosuspend_timeout);
pm_runtime_irq_safe(&pdev->dev);
pm_runtime_get_sync(&pdev->dev);
Right, now runtime PM knows that the device is enabled and alive prior
to that pm_runtime_get_sync() call, and it will _not_ call the runtime
resume hooks.
However, this breaks beaglebone, because the device is disabled when
this driver probes. So, we have exactly the opposite problem here -
the device is disabled, but runtime PM thinks it is enabled.
The _two_ problems are precisely the same problem: the runtime PM state
does not accurately reflect the actual hardware state - again, as required
by the runtime PM documentation. The only sane solution to this is to
ensure that the hardware _is_ in a known state prior to enabling runtime
PM.
How do we do that? Well, the clue is in the bus layer runtime resume
handler - that's what is missing from the beaglebone situation.
Calling this before calling pm_runtime_set_active() gets the hardware
into a known state (enabled), and we then tell the runtime PM code
that the harware _is_ enabled. Now, runtime PM can be sure what the
initial state is, and everything works.
What's the longer term answer?
Well, I _bet_ all OMAP drivers are doing something along the lines of:
pm_runtime_enable(dev);
pm_runtime_get(dev);
in their probe functions.
PCI already solved this problem - partly because it has far too many drivers
to convert. I took that solution over to the AMBA bus layer, because I
didn't want to have a flag day for all the drivers to convert over in one
massive patch. What is this solution?
You get the bus layer to handle the initial setup of runtime PM state like
this - in OMAP's case:
omap_device_enable(pdev);
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
You do this prior to calling the device probe function. If the device
probe fails, then you can undo those actions. You also need to undo them
when the device is unbound from the driver:
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_put_noidle(&pdev->dev);
(It is probably dangerous to call omap_device_disable() here for certain
devices...)
This gets rid of all that driver specific runtime PM initialization, with
questionable starting state. It also means that your devices all get
runtime PM support in so far as if they're bound to a driver, they will
be runtime PM resumed, and when unbound, they will be runtime PM suspended.
However, it means that the driver has to do something to make runtime PM
work. In the probe function, just before it returns success, it has to
'put' that pm_runtime_get_noresume() reference to allow the device to
enter runtime PM states. And more importantly, on a remove call, it
_must_ balance that 'put' in the probe with an appropriate 'get' - no
exceptions to that.
And that is how we get to a sane state over runtime PM here, which will
work in every situation on every device, without throwing calls to
omap_device_enable() into every OMAP device driver.
This also has another advantage - by doing that, the OMAP specific
omap_device_enable() call ends up being in bus layer code, not driver
layer, which is the right place for it to be - after all, it's the
bus layer which is already handling that stuff in its runtime PM support
code.
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Russell King - ARM Linux @ 2012-10-12 18:54 UTC (permalink / raw)
To: Kevin Hilman
Cc: Sourav, Paul Walmsley, Felipe Balbi, gregkh, tony, linux-kernel,
santosh.shilimkar, linux-serial, linux-omap, linux-arm-kernel,
alan
In-Reply-To: <877gqv4lmt.fsf@deeprootsystems.com>
On Fri, Oct 12, 2012 at 10:59:22AM -0700, Kevin Hilman wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>
> > On Fri, Oct 12, 2012 at 09:35:54AM -0700, Kevin Hilman wrote:
> >> Sourav <sourav.poddar@ti.com> writes:
> >> > diff --git a/drivers/tty/serial/omap-serial.c
> >> > b/drivers/tty/serial/omap-serial.c
> >> > index 6ede6fd..3fbc7f7 100644
> >> > --- a/drivers/tty/serial/omap-serial.c
> >> > +++ b/drivers/tty/serial/omap-serial.c
> >> > @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
> >> > platform_device *pdev)
> >> > INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
> >> >
> >> > platform_set_drvdata(pdev, up);
> >> > + pm_runtime_set_active(&pdev->dev);
> >>
> >> NAK.
> >>
> >> This will obviously break platforms where the UARTs are not active
> >> before driver loads.
> >
> > I thought I had proposed a solution for this issue, which was this
> > sequence:
> >
> > omap_device_enable(dev);
> > pm_runtime_set_active(dev);
> > pm_runtime_enable(dev);
> >
> > Yes, I can understand people not liking the omap_device_enable()
> > there, but I also notice that the email suggesting that never got a
> > reply either - not even a "I tried this and it doesn't work" or "it
> > does work".
>
> Yes, that solution would work (though I didn't actually try it.)
>
> However, we can't use omap_device_enable() in the driver. We're trying
> to clean all the drivers of OMAP-specific APIs. That being said,
> something similar could be done in the device/board init code to ensure
> the UART HW is in the state that the driver is expecting it, but again,
> that would just mask the real problem which is that a (re)init of the
> console UART on 2420/n800 is causing output to disappear.
See my more expansive suggestion just posted.
Whatever way, this discrepancy between runtime PM state and actual device
state is what is biting you, and it is that which needs fixing. It's
fairly easy to fix given the right design, one which several other bus
types are already using.
Given the route that OMAP went down when adopting runtime PM, it's going
to be a big change across many drivers, because there's no way to gradually
transition them, but that's unfortunately one of the results of ignoring
requirements of the layers being used. Sooner or later the oversights
come back to haunt. Just make sure it's not the ghost of Jaws.
^ permalink raw reply
* RE: 8250_early for big-endian
From: Noam Camus @ 2012-10-12 19:08 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <20121012195102.5781c7d4@pyramind.ukuu.org.uk>
Sorry,but I wrote that I am using mmio32.
I thought it implies that I am using 32bit registers.
This is why I keep asking.
________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Friday, October 12, 2012 8:51 PM
To: Noam Camus
Cc: linux-serial@vger.kernel.org
Subject: Re: 8250_early for big-endian
> Of course swapping is a waste of cpu cycles.
> Workaround can be 3 further bytes offset.
> Thanks
>
> Back to my problem.
> How can 8250_early be enhanced to support big endian registers.
They are 8bit registers - I don't understand why you are asking the
question ?
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Kevin Hilman @ 2012-10-12 20:32 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Sourav, Paul Walmsley, Felipe Balbi, gregkh, tony, linux-kernel,
santosh.shilimkar, linux-serial, linux-omap, linux-arm-kernel,
alan
In-Reply-To: <20121012185426.GS28061@n2100.arm.linux.org.uk>
Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Fri, Oct 12, 2012 at 10:59:22AM -0700, Kevin Hilman wrote:
>> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>>
>> > On Fri, Oct 12, 2012 at 09:35:54AM -0700, Kevin Hilman wrote:
>> >> Sourav <sourav.poddar@ti.com> writes:
>> >> > diff --git a/drivers/tty/serial/omap-serial.c
>> >> > b/drivers/tty/serial/omap-serial.c
>> >> > index 6ede6fd..3fbc7f7 100644
>> >> > --- a/drivers/tty/serial/omap-serial.c
>> >> > +++ b/drivers/tty/serial/omap-serial.c
>> >> > @@ -1414,6 +1414,7 @@ static int __devinit serial_omap_probe(struct
>> >> > platform_device *pdev)
>> >> > INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
>> >> >
>> >> > platform_set_drvdata(pdev, up);
>> >> > + pm_runtime_set_active(&pdev->dev);
>> >>
>> >> NAK.
>> >>
>> >> This will obviously break platforms where the UARTs are not active
>> >> before driver loads.
>> >
>> > I thought I had proposed a solution for this issue, which was this
>> > sequence:
>> >
>> > omap_device_enable(dev);
>> > pm_runtime_set_active(dev);
>> > pm_runtime_enable(dev);
>> >
>> > Yes, I can understand people not liking the omap_device_enable()
>> > there, but I also notice that the email suggesting that never got a
>> > reply either - not even a "I tried this and it doesn't work" or "it
>> > does work".
>>
>> Yes, that solution would work (though I didn't actually try it.)
>>
>> However, we can't use omap_device_enable() in the driver. We're trying
>> to clean all the drivers of OMAP-specific APIs. That being said,
>> something similar could be done in the device/board init code to ensure
>> the UART HW is in the state that the driver is expecting it, but again,
>> that would just mask the real problem which is that a (re)init of the
>> console UART on 2420/n800 is causing output to disappear.
>
> See my more expansive suggestion just posted.
>
> Whatever way, this discrepancy between runtime PM state and actual device
> state is what is biting you, and it is that which needs fixing.
I'm not conviced (yet) that a mismatch is the root cause. Yes, that's
what the author of $SUBJECT patch assumed and stated, but I'm not
pursuaded.
If it's an improperly configured mux issue, then the UART will break
whenever the device is actually omap_device_enable'd, whether in the
driver or in the bus layer.
Kevin
^ permalink raw reply
* Re: 8250_early for big-endian
From: Alan Cox @ 2012-10-12 21:06 UTC (permalink / raw)
To: Noam Camus; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <264C179F799EF24AB26D5319053335E80CC30499A5@ezexch.ezchip.com>
On Fri, 12 Oct 2012 21:08:15 +0200
Noam Camus <noamc@ezchip.com> wrote:
> Sorry,but I wrote that I am using mmio32.
Yes but you also keep talking about endian-ness. The underlying registers
are 8bit so they are not 'endian' at all. They might be at the address
"base + 4 * offset + 3" perhaps ?
If so they to use mmio32 presumably you just need to set the base address
properly (ie 0xXXXXXXX3 or similar).
Do the values have to be read with 32bit operations and do they have to
be read on 32bi alignment ?
Alan
^ permalink raw reply
* Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
From: Tony Lindgren @ 2012-10-12 21:51 UTC (permalink / raw)
To: Kevin Hilman
Cc: Russell King - ARM Linux, Sourav, Paul Walmsley, Felipe Balbi,
gregkh, linux-kernel, santosh.shilimkar, linux-serial, linux-omap,
linux-arm-kernel, alan
In-Reply-To: <87lifbwhwd.fsf@deeprootsystems.com>
* Kevin Hilman <khilman@deeprootsystems.com> [121012 13:34]:
>
> I'm not conviced (yet) that a mismatch is the root cause. Yes, that's
> what the author of $SUBJECT patch assumed and stated, but I'm not
> pursuaded.
>
> If it's an improperly configured mux issue, then the UART will break
> whenever the device is actually omap_device_enable'd, whether in the
> driver or in the bus layer.
I tried booting n800 here with CONFIG_OMAP_MUX disabled, and no
change. Serial console output stops right when the console initializes.
Regards,
Tony
^ permalink raw reply
* RE: PL2303 Driver Issue
From: Reddy Kaveri, Praveen Kumar @ 2012-10-12 22:15 UTC (permalink / raw)
To: Greg KH; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <20121007144700.GA18178@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]
Hi Greg,
Thanks for your assistance.
I am using RXTXComm API for getting list of serial ports and available USB ports. I have attached the java code for your kind glance.
For some of the point of sales machines the telxon device is working.
If the connection is successful, the following information is displaying.
Bus 001 Device 003: ID 0557:2008 ATEN International Co. Ltd.
Language IDs: none (invalid length string descriptor 63; len=7)
If the connection is failed, the following information is displaying.
Bus 001 Device 002: ID 0557:2008 ATEN International Co. Ltd.
string descriptor 1 invalid (00 00; len=52)
string descriptor 2 invalid (00 00; len=48)
I am less proficient in linux. So kindly help me in resolving this issue.
What does it mean "string descriptor 1 invalid"?
Is it causing the problem. If so please let know how to resolve this.
Thanks & Regards,
Praveen Kumar Reddy K
Hewlett-Packard Company
#101, 1035 64 Ave SE ,Calgary, AB, T2H 2J7, CANADA
+1 403 692 7914 / Tel
+1 403 619 8242 / Mobile
praveen-kumar.reddy-kaveri@hp.com / Email
-----Original Message-----
From: Greg KH [mailto:greg@kroah.com]
Sent: Sunday, October 07, 2012 8:47 AM
To: Reddy Kaveri, Praveen Kumar
Cc: linux-serial@vger.kernel.org
Subject: Re: PL2303 Driver Issue
On Sat, Oct 06, 2012 at 06:33:19PM +0000, Reddy Kaveri, Praveen Kumar wrote:
> Hi,
>
> I am using Telxon PTC-710 device in linux environment and I am using
> RS232 Serial to USB converter. When I connect the device to USB port
> it is detecting and showing in the system log as "attached to
> ttyUSB0". I have written a java program to display all the available
> ports. But this program is displaying only ttyS0 &ttyS1. I have tried
> to set all permissions to the user as well as to port. Please help me
> if I am missing any configuration related to this driver.
You are going to have to modify your userspace program to know to look
for the ttyUSBX devices as well. There's nothing the kernel can do
about this.
As you wrote the program, how are you getting the list of serial devices
in the system?
greg k-h
[-- Attachment #2: SerialProgramming.java --]
[-- Type: application/octet-stream, Size: 1838 bytes --]
package com.marks.builder;
import gnu.io.CommPortIdentifier;
import gnu.io.PortInUseException;
import gnu.io.SerialPort;
import gnu.io.UnsupportedCommOperationException;
import java.io.IOException;
import java.io.OutputStream;
import java.util.Enumeration;
public class SerialProgramming {
static Enumeration portList;
static CommPortIdentifier portId;
static String messageString = "Hello, world!\n";
static SerialPort serialPort;
static OutputStream outputStream;
public static void main(String[] args) {
portList = CommPortIdentifier.getPortIdentifiers();
while (portList.hasMoreElements()) {
portId = (CommPortIdentifier) portList.nextElement();
System.out.println(portId.getName());
if (portId.getPortType() == CommPortIdentifier.PORT_SERIAL) {
if (portId.getName().equals("COM3")) {
//if (portId.getName().equals("/dev/term/a")) {
try {
serialPort = (SerialPort)
portId.open("SimpleWriteApp", 2000);
} catch (PortInUseException e) {}
try {
outputStream = serialPort.getOutputStream();
} catch (IOException e) {}
try {
serialPort.setSerialPortParams(9600,
SerialPort.DATABITS_8,
SerialPort.STOPBITS_1,
SerialPort.PARITY_NONE);
} catch (UnsupportedCommOperationException e) {}
try {
outputStream.write(messageString.getBytes());
} catch (IOException e) {}
}
}
}
}
}
^ permalink raw reply
* Re: PL2303 Driver Issue
From: Alan Cox @ 2012-10-12 22:30 UTC (permalink / raw)
To: Reddy Kaveri, Praveen Kumar; +Cc: Greg KH, linux-serial@vger.kernel.org
In-Reply-To: <523E9909A6DFCF469669581BEB8DA7AA5692A2B7@G2W2441.americas.hpqcorp.net>
On Fri, 12 Oct 2012 22:15:08 +0000
"Reddy Kaveri, Praveen Kumar" <praveen-kumar.reddy-kaveri@hp.com> wrote:
> Hi Greg,
>
> Thanks for your assistance.
>
> I am using RXTXComm API for getting list of serial ports and available USB ports. I have attached the java code for your kind glance.
> For some of the point of sales machines the telxon device is working.
Make sure your Java implementation rxtxcomm isn't too stupid to look for
USB ports. There were endless problems with some old versions of rxtx with
JMRI and USB serial ports.
Also as the same user try stty -a </dev/ttyUSB0. That will if it's
behaving sensibly dump the terminal state (speed etc) on your screen.
If that works you've almost certainly got a problem with your java.
^ permalink raw reply
* RE: 8250_early for big-endian
From: Noam Camus @ 2012-10-13 6:43 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <20121012220604.3f99d003@pyramind.ukuu.org.uk>
On Fri, 12 Oct 2012 21:11:06 +0200
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Do the values have to be read with 32bit operations and do they have to
> be read on 32bi alignment ?
Yes I do need the 32bit alignment but I do not need 32bit operations.
So using mmio (instead of mmio32) with regshift equals 2 worked for me.
However 8250_early got no configurable regshift.
So it needs to be added.
Alternatively I can use regshift equals 0 and redefine locally all offsets from serial_reg.h e.g. UART_LST, UART_IER...
What do you think is the way to work this regshift?
BTW: no more endian-ness from my side.
________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Friday, October 12, 2012 11:06 PM
To: Noam Camus
Cc: linux-serial@vger.kernel.org
Subject: Re: 8250_early for big-endian
On Fri, 12 Oct 2012 21:08:15 +0200
Noam Camus <noamc@ezchip.com> wrote:
> Sorry,but I wrote that I am using mmio32.
Yes but you also keep talking about endian-ness. The underlying registers
are 8bit so they are not 'endian' at all. They might be at the address
"base + 4 * offset + 3" perhaps ?
If so they to use mmio32 presumably you just need to set the base address
properly (ie 0xXXXXXXX3 or similar).
Do the values have to be read with 32bit operations and do they have to
be read on 32bi alignment ?
Alan
^ permalink raw reply
* Re: 8250_early for big-endian
From: Alan Cox @ 2012-10-13 13:52 UTC (permalink / raw)
To: Noam Camus; +Cc: linux-serial@vger.kernel.org
In-Reply-To: <264C179F799EF24AB26D5319053335E80CC30499A9@ezexch.ezchip.com>
On Sat, 13 Oct 2012 08:43:02 +0200
Noam Camus <noamc@ezchip.com> wrote:
> On Fri, 12 Oct 2012 21:11:06 +0200
> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>
> > Do the values have to be read with 32bit operations and do they have to
> > be read on 32bi alignment ?
>
> Yes I do need the 32bit alignment but I do not need 32bit operations.
> So using mmio (instead of mmio32) with regshift equals 2 worked for me.
>
> However 8250_early got no configurable regshift.
> So it needs to be added.
I would favour adding the regshift and I see no problem in doing that.
Alan
^ permalink raw reply
* order
From: Royaldoc International Globe @ 2012-10-11 16:23 UTC (permalink / raw)
Royaldoc International Globe,
103, Park son road,
Carlifonia,USA.
63-1-3522 2501
royaldoc@inbox.org.tw
We are interested in purchasing your products and I would like to
make an inquiry. can you inform me of your minimum order quantity? and
also your maximum order quantity, if there is any available sample do
let me know.
Sincerely,
Purchase Manager
Alan Smith
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox