| Message ID | 1234438768-20697-1-git-send-email-roman.tereshonkov@nokia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2bf450019410d15dbce63893d3f91e076a5a70c0 |
| Headers | show |
On Thu, Feb 12, 2009 at 12:39:28PM +0100, Tereshonkov Roman wrote: > The function sti_channel_write_trace can be run from process and interrupt > context. It has to be completed before other sti_channel_write_trace calls. > > Prevent sdti writing when SDTI module is not initialized. > > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> Acked-by: Felipe Balbi <felipe.balbi@nokia.com> > --- > drivers/misc/sti/sdti.c | 15 ++++++++++++--- > 1 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/misc/sti/sdti.c b/drivers/misc/sti/sdti.c > index 8c27504..31780de 100644 > --- a/drivers/misc/sti/sdti.c > +++ b/drivers/misc/sti/sdti.c > @@ -37,20 +37,25 @@ > static struct clk *sdti_fck, *sdti_ick; > void __iomem *sti_base, *sti_channel_base; > static DEFINE_SPINLOCK(sdti_lock); > +static int sdti_initialized; > > void sti_channel_write_trace(int len, int id, void *data, > unsigned int channel) > { > const u8 *tpntr = data; > + unsigned long flags; > > - spin_lock_irq(&sdti_lock); > + spin_lock_irqsave(&sdti_lock, flags); > + > + if (unlikely(!sdti_initialized)) > + goto skip; > > sti_channel_writeb(id, channel); > while (len--) > sti_channel_writeb(*tpntr++, channel); > sti_channel_flush(channel); > - > - spin_unlock_irq(&sdti_lock); > + skip: > + spin_unlock_irqrestore(&sdti_lock, flags); > } > EXPORT_SYMBOL(sti_channel_write_trace); > > @@ -117,6 +122,10 @@ static int __init omap_sdti_init(void) > /* Enable SDTI */ > sti_writel((1 << 31) | (i & 0x3FFFFFFF), SDTI_WINCTRL); > > + spin_lock_irq(&sdti_lock); > + sdti_initialized = 1; > + spin_unlock_irq(&sdti_lock); > + > i = sti_readl(SDTI_REVISION); > snprintf(buf, sizeof(buf), "OMAP SDTI support loaded (HW v%u.%u)\n", > (i >> 4) & 0x0f, i & 0x0f); > -- > 1.5.5.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
* Felipe Balbi <felipe.balbi@nokia.com> [090216 03:50]: > On Thu, Feb 12, 2009 at 12:39:28PM +0100, Tereshonkov Roman wrote: > > The function sti_channel_write_trace can be run from process and interrupt > > context. It has to be completed before other sti_channel_write_trace calls. > > > > Prevent sdti writing when SDTI module is not initialized. Roman, can you please create a patch against the mainline tree and send this driver to LKML for integration? I think pretty much the only thing you should do is get rid of the OMAP_TAG_STI_CONSOLE and use get that option from kernel command line like all the other consoles do. Regards, Tony > > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> > > Acked-by: Felipe Balbi <felipe.balbi@nokia.com> > > > --- > > drivers/misc/sti/sdti.c | 15 ++++++++++++--- > > 1 files changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/misc/sti/sdti.c b/drivers/misc/sti/sdti.c > > index 8c27504..31780de 100644 > > --- a/drivers/misc/sti/sdti.c > > +++ b/drivers/misc/sti/sdti.c > > @@ -37,20 +37,25 @@ > > static struct clk *sdti_fck, *sdti_ick; > > void __iomem *sti_base, *sti_channel_base; > > static DEFINE_SPINLOCK(sdti_lock); > > +static int sdti_initialized; > > > > void sti_channel_write_trace(int len, int id, void *data, > > unsigned int channel) > > { > > const u8 *tpntr = data; > > + unsigned long flags; > > > > - spin_lock_irq(&sdti_lock); > > + spin_lock_irqsave(&sdti_lock, flags); > > + > > + if (unlikely(!sdti_initialized)) > > + goto skip; > > > > sti_channel_writeb(id, channel); > > while (len--) > > sti_channel_writeb(*tpntr++, channel); > > sti_channel_flush(channel); > > - > > - spin_unlock_irq(&sdti_lock); > > + skip: > > + spin_unlock_irqrestore(&sdti_lock, flags); > > } > > EXPORT_SYMBOL(sti_channel_write_trace); > > > > @@ -117,6 +122,10 @@ static int __init omap_sdti_init(void) > > /* Enable SDTI */ > > sti_writel((1 << 31) | (i & 0x3FFFFFFF), SDTI_WINCTRL); > > > > + spin_lock_irq(&sdti_lock); > > + sdti_initialized = 1; > > + spin_unlock_irq(&sdti_lock); > > + > > i = sti_readl(SDTI_REVISION); > > snprintf(buf, sizeof(buf), "OMAP SDTI support loaded (HW v%u.%u)\n", > > (i >> 4) & 0x0f, i & 0x0f); > > -- > > 1.5.5.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > balbi > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 27, 2009 at 10:35:25AM -0800, Tony Lindgren wrote: > * Felipe Balbi <felipe.balbi@nokia.com> [090216 03:50]: > > On Thu, Feb 12, 2009 at 12:39:28PM +0100, Tereshonkov Roman wrote: > > > The function sti_channel_write_trace can be run from process and interrupt > > > context. It has to be completed before other sti_channel_write_trace calls. > > > > > > Prevent sdti writing when SDTI module is not initialized. > > Roman, can you please create a patch against the mainline tree and send > this driver to LKML for integration? I think pretty much the only thing > you should do is get rid of the OMAP_TAG_STI_CONSOLE and use get that > option from kernel command line like all the other consoles do. Adding Juha to Cc as he had some comments about the OMAP_TAG_STI_CONSOLE regarding our products. Juha could you comment again what were the problems on starting the sti console based on console= command line ??
On Fri, Feb 27, 2009 at 09:00:53PM +0200, Felipe Balbi wrote: > Juha could you comment again what were the problems on starting the sti > console based on console= command line ?? STI console is enabled based on runtime decisions by the bootloader. The kernel command line is unusable for us, because there is no way to append to the line. We don't want to remove the possibility how having the user create a custom kernel with a custom command line. Cheers, Juha -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28/02/09 11:08 +0100, ext Juha Yrjola wrote: > On Fri, Feb 27, 2009 at 09:00:53PM +0200, Felipe Balbi wrote: > > > Juha could you comment again what were the problems on starting the sti > > console based on console= command line ?? > > STI console is enabled based on runtime decisions by the bootloader. The > kernel command line is unusable for us, because there is no way to append to > the line. We don't want to remove the possibility how having the user > create a custom kernel with a custom command line. *I* don't have a problem with having the bootloader always define the cmdline, you are the one against it :) /lauri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 02, 2009 at 10:27:50AM +0200, Lauri Leukkunen wrote: > > STI console is enabled based on runtime decisions by the bootloader. The > > kernel command line is unusable for us, because there is no way to append to > > the line. We don't want to remove the possibility how having the user > > create a custom kernel with a custom command line. > > *I* don't have a problem with having the bootloader always define the > cmdline, you are the one against it :) If we did that, how would the kernel flashing process go? Cheers, Juha -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/03/09 12:31 +0100, ext Juha Yrjola wrote: > On Mon, Mar 02, 2009 at 10:27:50AM +0200, Lauri Leukkunen wrote: > > > > STI console is enabled based on runtime decisions by the bootloader. The > > > kernel command line is unusable for us, because there is no way to append to > > > the line. We don't want to remove the possibility how having the user > > > create a custom kernel with a custom command line. > > > > *I* don't have a problem with having the bootloader always define the > > cmdline, you are the one against it :) > > If we did that, how would the kernel flashing process go? flasher needs to support updating the cmdline stored in the CAL, maybe with: flasher -b"cmdline" -f Then you just ignore whatever the kernel builtin cmdline is. So no need to change how the kernel is flashed. If a person is anyway modifying the cmdline, he might as well do it using the above mentioned command instead of hardcoding it into the kernel. This way he can experiment with different cmdlines without having to recompile and reflash the kernel. Maybe a -b"default" -f could be used to put back the default (perhaps also found from the CAL area?). This would require for the default cmdline to be written to CAL at manufacturing time, or be carried by the bootloader binary itself. Either way would be ok to me. /lauri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/misc/sti/sdti.c b/drivers/misc/sti/sdti.c index 8c27504..31780de 100644 --- a/drivers/misc/sti/sdti.c +++ b/drivers/misc/sti/sdti.c @@ -37,20 +37,25 @@ static struct clk *sdti_fck, *sdti_ick; void __iomem *sti_base, *sti_channel_base; static DEFINE_SPINLOCK(sdti_lock); +static int sdti_initialized; void sti_channel_write_trace(int len, int id, void *data, unsigned int channel) { const u8 *tpntr = data; + unsigned long flags; - spin_lock_irq(&sdti_lock); + spin_lock_irqsave(&sdti_lock, flags); + + if (unlikely(!sdti_initialized)) + goto skip; sti_channel_writeb(id, channel); while (len--) sti_channel_writeb(*tpntr++, channel); sti_channel_flush(channel); - - spin_unlock_irq(&sdti_lock); + skip: + spin_unlock_irqrestore(&sdti_lock, flags); } EXPORT_SYMBOL(sti_channel_write_trace); @@ -117,6 +122,10 @@ static int __init omap_sdti_init(void) /* Enable SDTI */ sti_writel((1 << 31) | (i & 0x3FFFFFFF), SDTI_WINCTRL); + spin_lock_irq(&sdti_lock); + sdti_initialized = 1; + spin_unlock_irq(&sdti_lock); + i = sti_readl(SDTI_REVISION); snprintf(buf, sizeof(buf), "OMAP SDTI support loaded (HW v%u.%u)\n", (i >> 4) & 0x0f, i & 0x0f);
The function sti_channel_write_trace can be run from process and interrupt context. It has to be completed before other sti_channel_write_trace calls. Prevent sdti writing when SDTI module is not initialized. Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> --- drivers/misc/sti/sdti.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-)