[PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Roman Tereshonkov]

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(-)

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

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Felipe Balbi]

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

-- 
balbi

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Tony Lindgren]

* 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

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Felipe Balbi]

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 ??

-- 
balbi

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Juha Yrjola]

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

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Lauri Leukkunen]

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

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Juha Yrjola]

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

Re: [PATCH] OMAP3: SDTI: Prevent access to sdti writing if module is not initialized.

[Lauri Leukkunen]

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