[PATCH v3 0/2] omap: serial: handle abort on empty rx fifo read

[PATCH v3 0/2] omap: serial: handle abort on empty rx fifo read

[Vikram Pandita]

introduce OMAP3_HAS_UART_NO_EMPTY_FIFO_READ feature

this feature is set for omap3630 and omap4 currently as 
empty uart rx fifo read causes an abort

check on this feature on omap3630 and omap4 for now and extend for future
vairants in future

Patch history:
V1: initial implementation
	http://patchwork.kernel.org/patch/60785/
	http://patchwork.kernel.org/patch/60786/

V2: incorporate review comments from Alan Cox
	http://patchwork.kernel.org/patch/60785/
	to remove 8250.c file changes by override serial_in
	No 8250 driver change required now

V3: incorporate review comments
	khilman: make function override only for affected silicons
	nishant m: interoduce has_feature check, rather than cpu_is
	anand g: minor commit message change

Vikram Pandita (2):
  omap: introduce uart_no_empty_fifo_read feature
  omap: serial: fix non-empty uart fifo read abort

 arch/arm/mach-omap2/id.c              |    7 +++++++
 arch/arm/mach-omap2/serial.c          |   21 +++++++++++++++++++++
 arch/arm/plat-omap/include/plat/cpu.h |    2 ++
 3 files changed, 30 insertions(+), 0 deletions(-)

[PATCH v3 1/2] omap: introduce uart_no_empty_fifo_read feature

[Vikram Pandita]

Interoduce omap feature OMAP3_HAS_UART_NO_EMPTY_FIFO_READ

On omap3630/omap4 an empty fifo read causes a crash

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Ack-by: Menon, Nishanth <nm@ti.com>
---
 arch/arm/mach-omap2/id.c              |    7 +++++++
 arch/arm/plat-omap/include/plat/cpu.h |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index f48a4b2..3e266cd 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -176,6 +176,12 @@ void __init omap3_check_features(void)
 	OMAP3_CHECK_FEATURE(status, NEON);
 	OMAP3_CHECK_FEATURE(status, ISP);
 
+	/* On omap3630 and omap4: UART empty rx fifo read aborts */
+	if (cpu_is_omap3630())
+		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
+	if (cpu_is_omap44xx())
+		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
+
 	/*
 	 * TODO: Get additional info (where applicable)
 	 *       e.g. Size of L2 cache.
@@ -316,6 +322,7 @@ void __init omap3_cpuinfo(void)
 	OMAP3_SHOW_FEATURE(sgx);
 	OMAP3_SHOW_FEATURE(neon);
 	OMAP3_SHOW_FEATURE(isp);
+	OMAP3_SHOW_FEATURE(uart_no_empty_fifo_read);
 
 	printk(")\n");
 }
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 2e17890..c32f015 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -497,6 +497,7 @@ extern u32 omap3_features;
 #define OMAP3_HAS_SGX			BIT(2)
 #define OMAP3_HAS_NEON			BIT(3)
 #define OMAP3_HAS_ISP			BIT(4)
+#define OMAP3_HAS_UART_NO_EMPTY_FIFO_READ	BIT(5)
 
 #define OMAP3_HAS_FEATURE(feat,flag)			\
 static inline unsigned int omap3_has_ ##feat(void)	\
@@ -509,5 +510,6 @@ OMAP3_HAS_FEATURE(sgx, SGX)
 OMAP3_HAS_FEATURE(iva, IVA)
 OMAP3_HAS_FEATURE(neon, NEON)
 OMAP3_HAS_FEATURE(isp, ISP)
+OMAP3_HAS_FEATURE(uart_no_empty_fifo_read, UART_NO_EMPTY_FIFO_READ)
 
 #endif
-- 
1.6.5.1.69.g36942

[PATCH v3 2/2] omap: serial: fix non-empty uart fifo read abort

[Vikram Pandita]

OMAP3630 and OMAP4430 UART IP blocks have a restriction wrt RX FIFO.
Empty RX fifo read causes an abort. OMAP1/2/3 do not have this restriction.

Override the default 8250 read handler: mem_serial_in()
by a custom handler: serial_in_8250()

serial_in_8250() makes sure that RX fifo is not read when empty,
on omap4 and 3630 silicons only

tested on zoom3(3630) board

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 arch/arm/mach-omap2/serial.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 2e17b57..600e4d2 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -572,6 +572,23 @@ static struct omap_uart_state omap_uart[] = {
 #endif
 };
 
+/*
+ * Override the default 8250 read handler: mem_serial_in()
+ * Empty RX fifo read causes an abort on omap3630 and omap4
+ * This function makes sure that an empty rx fifo is not read on these silicons
+ * (OMAP1/2/3 are not affected)
+ */
+static unsigned int serial_in_override(struct uart_port *up, int offset)
+{
+	if (UART_RX == offset) {
+		unsigned int lsr;
+		lsr = serial_read_reg(omap_uart[up->line].p, UART_LSR);
+		if (!(lsr & UART_LSR_DR))
+			return -EPERM;
+	}
+	return serial_read_reg(omap_uart[up->line].p, offset);
+}
+
 void __init omap_serial_early_init(void)
 {
 	int i;
@@ -627,6 +644,10 @@ void __init omap_serial_early_init(void)
 		if (cpu_is_omap44xx())
 			p->irq += 32;
 
+		/* Do not read empty UART fifo on omap3630/omap4 */
+		if (omap3_has_uart_no_empty_fifo_read())
+			p->serial_in = serial_in_override;
+
 		omap_uart_enable_clocks(uart);
 	}
 }
-- 
1.6.5.1.69.g36942

Re: [PATCH v3 1/2] omap: introduce uart_no_empty_fifo_read feature

[Nishanth Menon]

Vikram Pandita had written, on 11/20/2009 10:02 AM, the following:
> Interoduce omap feature OMAP3_HAS_UART_NO_EMPTY_FIFO_READ
    ^^^^^^^^^ <- you meant introduce
> 
> On omap3630/omap4 an empty fifo read causes a crash
> 
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> Ack-by: Menon, Nishanth <nm@ti.com>
    ^^^^^ <- :P nope you dont have my Acked-by until you change this to 
Acked from Ack ;)..

Thanks for the simpler patch.

> ---
>  arch/arm/mach-omap2/id.c              |    7 +++++++
>  arch/arm/plat-omap/include/plat/cpu.h |    2 ++
>  2 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> index f48a4b2..3e266cd 100644
> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -176,6 +176,12 @@ void __init omap3_check_features(void)
>  	OMAP3_CHECK_FEATURE(status, NEON);
>  	OMAP3_CHECK_FEATURE(status, ISP);
>  
> +	/* On omap3630 and omap4: UART empty rx fifo read aborts */
> +	if (cpu_is_omap3630())
> +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
> +	if (cpu_is_omap44xx())
> +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
> +
>  	/*
>  	 * TODO: Get additional info (where applicable)
>  	 *       e.g. Size of L2 cache.
> @@ -316,6 +322,7 @@ void __init omap3_cpuinfo(void)
>  	OMAP3_SHOW_FEATURE(sgx);
>  	OMAP3_SHOW_FEATURE(neon);
>  	OMAP3_SHOW_FEATURE(isp);
> +	OMAP3_SHOW_FEATURE(uart_no_empty_fifo_read);
>  
>  	printk(")\n");
>  }
> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
> index 2e17890..c32f015 100644
> --- a/arch/arm/plat-omap/include/plat/cpu.h
> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> @@ -497,6 +497,7 @@ extern u32 omap3_features;
>  #define OMAP3_HAS_SGX			BIT(2)
>  #define OMAP3_HAS_NEON			BIT(3)
>  #define OMAP3_HAS_ISP			BIT(4)
> +#define OMAP3_HAS_UART_NO_EMPTY_FIFO_READ	BIT(5)
>  
>  #define OMAP3_HAS_FEATURE(feat,flag)			\
>  static inline unsigned int omap3_has_ ##feat(void)	\
> @@ -509,5 +510,6 @@ OMAP3_HAS_FEATURE(sgx, SGX)
>  OMAP3_HAS_FEATURE(iva, IVA)
>  OMAP3_HAS_FEATURE(neon, NEON)
>  OMAP3_HAS_FEATURE(isp, ISP)
> +OMAP3_HAS_FEATURE(uart_no_empty_fifo_read, UART_NO_EMPTY_FIFO_READ)
>  
>  #endif


-- 
Regards,
Nishanth Menon

RE: [PATCH v3 1/2] omap: introduce uart_no_empty_fifo_read feature

[Aguirre, Sergio]

 

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Nishanth Menon
> Sent: Friday, November 20, 2009 10:10 AM
> To: Pandita, Vikram
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH v3 1/2] omap: introduce 
> uart_no_empty_fifo_read feature
> 
> Vikram Pandita had written, on 11/20/2009 10:02 AM, the following:
> > Interoduce omap feature OMAP3_HAS_UART_NO_EMPTY_FIFO_READ
>     ^^^^^^^^^ <- you meant introduce
> > 
> > On omap3630/omap4 an empty fifo read causes a crash
> > 
> > Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> > Ack-by: Menon, Nishanth <nm@ti.com>
>     ^^^^^ <- :P nope you dont have my Acked-by until you 
> change this to 
> Acked from Ack ;)..
> 
> Thanks for the simpler patch.
> 
> > ---
> >  arch/arm/mach-omap2/id.c              |    7 +++++++
> >  arch/arm/plat-omap/include/plat/cpu.h |    2 ++
> >  2 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> > index f48a4b2..3e266cd 100644
> > --- a/arch/arm/mach-omap2/id.c
> > +++ b/arch/arm/mach-omap2/id.c
> > @@ -176,6 +176,12 @@ void __init omap3_check_features(void)
> >  	OMAP3_CHECK_FEATURE(status, NEON);
> >  	OMAP3_CHECK_FEATURE(status, ISP);
> >  
> > +	/* On omap3630 and omap4: UART empty rx fifo read aborts */
> > +	if (cpu_is_omap3630())
> > +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
> > +	if (cpu_is_omap44xx())
> > +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
> > +

Probably not something ot be attached in this patch, but...

I'm a bit curious about something:

Why touching omap3_features in OMAP4?

Isn't there a omap4_features?

Or even better, an omap_features?

Regards,
Sergio

> >  	/*
> >  	 * TODO: Get additional info (where applicable)
> >  	 *       e.g. Size of L2 cache.
> > @@ -316,6 +322,7 @@ void __init omap3_cpuinfo(void)
> >  	OMAP3_SHOW_FEATURE(sgx);
> >  	OMAP3_SHOW_FEATURE(neon);
> >  	OMAP3_SHOW_FEATURE(isp);
> > +	OMAP3_SHOW_FEATURE(uart_no_empty_fifo_read);
> >  
> >  	printk(")\n");
> >  }
> > diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
> b/arch/arm/plat-omap/include/plat/cpu.h
> > index 2e17890..c32f015 100644
> > --- a/arch/arm/plat-omap/include/plat/cpu.h
> > +++ b/arch/arm/plat-omap/include/plat/cpu.h
> > @@ -497,6 +497,7 @@ extern u32 omap3_features;
> >  #define OMAP3_HAS_SGX			BIT(2)
> >  #define OMAP3_HAS_NEON			BIT(3)
> >  #define OMAP3_HAS_ISP			BIT(4)
> > +#define OMAP3_HAS_UART_NO_EMPTY_FIFO_READ	BIT(5)
> >  
> >  #define OMAP3_HAS_FEATURE(feat,flag)			\
> >  static inline unsigned int omap3_has_ ##feat(void)	\
> > @@ -509,5 +510,6 @@ OMAP3_HAS_FEATURE(sgx, SGX)
> >  OMAP3_HAS_FEATURE(iva, IVA)
> >  OMAP3_HAS_FEATURE(neon, NEON)
> >  OMAP3_HAS_FEATURE(isp, ISP)
> > +OMAP3_HAS_FEATURE(uart_no_empty_fifo_read, UART_NO_EMPTY_FIFO_READ)
> >  
> >  #endif
> 
> 
> -- 
> Regards,
> Nishanth Menon
> --
> 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
> 
> 

FEATURES - is it good enough (was Re: [PATCH v3 1/2] omap: introduce uart_no_empty_fifo_read feature)

[Nishanth Menon]

Aguirre, Sergio had written, on 11/20/2009 10:14 AM, the following:
>  
> 
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org 
>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Nishanth Menon
>> Sent: Friday, November 20, 2009 10:10 AM
>> To: Pandita, Vikram
>> Cc: linux-omap@vger.kernel.org
>> Subject: Re: [PATCH v3 1/2] omap: introduce 
>> uart_no_empty_fifo_read feature
>>
>> Vikram Pandita had written, on 11/20/2009 10:02 AM, the following:
>>> Interoduce omap feature OMAP3_HAS_UART_NO_EMPTY_FIFO_READ
>>     ^^^^^^^^^ <- you meant introduce
>>> On omap3630/omap4 an empty fifo read causes a crash
>>>
>>> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
>>> Ack-by: Menon, Nishanth <nm@ti.com>
>>     ^^^^^ <- :P nope you dont have my Acked-by until you 
>> change this to 
>> Acked from Ack ;)..
>>
>> Thanks for the simpler patch.
>>
>>> ---
>>>  arch/arm/mach-omap2/id.c              |    7 +++++++
>>>  arch/arm/plat-omap/include/plat/cpu.h |    2 ++
>>>  2 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
>>> index f48a4b2..3e266cd 100644
>>> --- a/arch/arm/mach-omap2/id.c
>>> +++ b/arch/arm/mach-omap2/id.c
>>> @@ -176,6 +176,12 @@ void __init omap3_check_features(void)
>>>  	OMAP3_CHECK_FEATURE(status, NEON);
>>>  	OMAP3_CHECK_FEATURE(status, ISP);
>>>  
>>> +	/* On omap3630 and omap4: UART empty rx fifo read aborts */
>>> +	if (cpu_is_omap3630())
>>> +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
>>> +	if (cpu_is_omap44xx())
>>> +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
>>> +
> 
> Probably not something ot be attached in this patch, but...
> 
> I'm a bit curious about something:
> 
> Why touching omap3_features in OMAP4?
> 
> Isn't there a omap4_features?
> 
> Or even better, an omap_features?

yep, that is a limitation in today's FEATUREs design which we have to 
fix. I see issues such as:
a) ability to scale across silicons (OMAP1 - 4)
b) differentiate between silicon feature vs erratas
c) limit to 32 bits.. meaning, we may want to think something else at a 
later point..

FEATUREs is the first step in this.. we have more to go for a uniform system

> 
> Regards,
> Sergio
> 
>>>  	/*
>>>  	 * TODO: Get additional info (where applicable)
>>>  	 *       e.g. Size of L2 cache.
>>> @@ -316,6 +322,7 @@ void __init omap3_cpuinfo(void)
>>>  	OMAP3_SHOW_FEATURE(sgx);
>>>  	OMAP3_SHOW_FEATURE(neon);
>>>  	OMAP3_SHOW_FEATURE(isp);
>>> +	OMAP3_SHOW_FEATURE(uart_no_empty_fifo_read);
>>>  
>>>  	printk(")\n");
>>>  }
>>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
>> b/arch/arm/plat-omap/include/plat/cpu.h
>>> index 2e17890..c32f015 100644
>>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>>> @@ -497,6 +497,7 @@ extern u32 omap3_features;
>>>  #define OMAP3_HAS_SGX			BIT(2)
>>>  #define OMAP3_HAS_NEON			BIT(3)
>>>  #define OMAP3_HAS_ISP			BIT(4)
>>> +#define OMAP3_HAS_UART_NO_EMPTY_FIFO_READ	BIT(5)
>>>  
>>>  #define OMAP3_HAS_FEATURE(feat,flag)			\
>>>  static inline unsigned int omap3_has_ ##feat(void)	\
>>> @@ -509,5 +510,6 @@ OMAP3_HAS_FEATURE(sgx, SGX)
>>>  OMAP3_HAS_FEATURE(iva, IVA)
>>>  OMAP3_HAS_FEATURE(neon, NEON)
>>>  OMAP3_HAS_FEATURE(isp, ISP)
>>> +OMAP3_HAS_FEATURE(uart_no_empty_fifo_read, UART_NO_EMPTY_FIFO_READ)
>>>  
>>>  #endif
>>
>> -- 
>> Regards,
>> Nishanth Menon
>> --
>> 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
>>


-- 
Regards,
Nishanth Menon

RE: FEATURES - is it good enough (was Re: [PATCH v3 1/2] omap: introduce uart_no_empty_fifo_read feature)

[Shilimkar, Santosh]

Started reading threads today....

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Menon, Nishanth
> Sent: Friday, November 20, 2009 9:47 PM
> To: Aguirre, Sergio
> Cc: Pandita, Vikram; linux-omap@vger.kernel.org
> Subject: FEATURES - is it good enough (was Re: [PATCH v3 1/2] omap:
> introduce uart_no_empty_fifo_read feature)
> 
> Aguirre, Sergio had written, on 11/20/2009 10:14 AM, the following:
> >
> >
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org
> >> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Nishanth
> Menon
> >> Sent: Friday, November 20, 2009 10:10 AM
> >> To: Pandita, Vikram
> >> Cc: linux-omap@vger.kernel.org
> >> Subject: Re: [PATCH v3 1/2] omap: introduce
> >> uart_no_empty_fifo_read feature
> >>
> >> Vikram Pandita had written, on 11/20/2009 10:02 AM, the
> following:
> >>> Interoduce omap feature OMAP3_HAS_UART_NO_EMPTY_FIFO_READ
> >>     ^^^^^^^^^ <- you meant introduce
> >>> On omap3630/omap4 an empty fifo read causes a crash
> >>>
> >>> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> >>> Ack-by: Menon, Nishanth <nm@ti.com>
> >>     ^^^^^ <- :P nope you dont have my Acked-by until you
> >> change this to
> >> Acked from Ack ;)..
> >>
> >> Thanks for the simpler patch.
> >>
> >>> ---
> >>>  arch/arm/mach-omap2/id.c              |    7 +++++++
> >>>  arch/arm/plat-omap/include/plat/cpu.h |    2 ++
> >>>  2 files changed, 9 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> >>> index f48a4b2..3e266cd 100644
> >>> --- a/arch/arm/mach-omap2/id.c
> >>> +++ b/arch/arm/mach-omap2/id.c
> >>> @@ -176,6 +176,12 @@ void __init omap3_check_features(void)
> >>>  	OMAP3_CHECK_FEATURE(status, NEON);
> >>>  	OMAP3_CHECK_FEATURE(status, ISP);
> >>>
> >>> +	/* On omap3630 and omap4: UART empty rx fifo read aborts */
> >>> +	if (cpu_is_omap3630())
> >>> +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
> >>> +	if (cpu_is_omap44xx())
> >>> +		omap3_features |= OMAP3_HAS_UART_NO_EMPTY_FIFO_READ;
> >>> +
> >
> > Probably not something ot be attached in this patch, but...
> >
> > I'm a bit curious about something:
> >
> > Why touching omap3_features in OMAP4?
> >
> > Isn't there a omap4_features?
> >
> > Or even better, an omap_features?

This "is_feature" suppose to take care of Errata's also, is it?
This is errata more than a feature..... We better differentiate in this regard

Regards,
Santosh

Re: FEATURES - is it good enough

[Kevin Hilman]

"Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:

[...]

>> >
>> > Probably not something ot be attached in this patch, but...
>> >
>> > I'm a bit curious about something:
>> >
>> > Why touching omap3_features in OMAP4?
>> >
>> > Isn't there a omap4_features?
>> >
>> > Or even better, an omap_features?
>
> This "is_feature" suppose to take care of Errata's also, is it?

"It's not a bug it's a feature." :)

> This is errata more than a feature..... We better differentiate in
> this regard

I agree, I have a hard time calling this empty fifo read fault a
"feature."  We need a similar thing for errata.

Kevin

Re: FEATURES - is it good enough

[Nishanth Menon]

Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
> "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> 
> [...]
> 
>>>> Probably not something ot be attached in this patch, but...
>>>>
>>>> I'm a bit curious about something:
>>>>
>>>> Why touching omap3_features in OMAP4?
>>>>
>>>> Isn't there a omap4_features?
>>>>
>>>> Or even better, an omap_features?
>> This "is_feature" suppose to take care of Errata's also, is it?
> 
> "It's not a bug it's a feature." :)
Bug. Santosh pointed out internally to h/w discussion which clearly 
shows this as a h/w limitation. (thanks santosh)

> 
>> This is errata more than a feature..... We better differentiate in
>> this regard
> 
> I agree, I have a hard time calling this empty fifo read fault a
> "feature."  We need a similar thing for errata.
Agreed. This is a classic example why we need a common errata handling 
mechanism scalable across silicon variants on an IP basis. two problems 
in front of us:
a) what do we want to do with 8250 workaround needed for omap3630 and 
omap4? can we go ahead with features marking it clearly as a "misuse of 
features for the time being"
b) a common silicon errata handling mechanism: Does anyone have 
proposals for this? I can see it help in numerous places in our code 
today and will help readability of the code instead of getting the risk 
of "feature not a bug" misread.. ;)..

-- 
Regards,
Nishanth Menon

RE: FEATURES - is it good enough

[Premi, Sanjeev]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Menon, Nishanth
> Sent: Saturday, November 21, 2009 12:54 AM
> To: Kevin Hilman
> Cc: Shilimkar, Santosh; Aguirre, Sergio; Pandita, Vikram; 
> linux-omap@vger.kernel.org
> Subject: Re: FEATURES - is it good enough
> 
> Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
> > "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> > 
> > [...]
> > 
> >>>> Probably not something ot be attached in this patch, but...
> >>>>
> >>>> I'm a bit curious about something:
> >>>>
> >>>> Why touching omap3_features in OMAP4?
> >>>>
> >>>> Isn't there a omap4_features?
> >>>>
> >>>> Or even better, an omap_features?
> >> This "is_feature" suppose to take care of Errata's also, is it?
> > 
> > "It's not a bug it's a feature." :)
> Bug. Santosh pointed out internally to h/w discussion which clearly 
> shows this as a h/w limitation. (thanks santosh)
> 
> > 
> >> This is errata more than a feature..... We better differentiate in
> >> this regard
> > 
> > I agree, I have a hard time calling this empty fifo read fault a
> > "feature."  We need a similar thing for errata.
> Agreed. This is a classic example why we need a common errata 
> handling 
> mechanism scalable across silicon variants on an IP basis. 
> two problems 
> in front of us:
> a) what do we want to do with 8250 workaround needed for omap3630 and 
> omap4? can we go ahead with features marking it clearly as a 
> "misuse of 
> features for the time being"
> b) a common silicon errata handling mechanism: Does anyone have 
> proposals for this? I can see it help in numerous places in our code 
> today and will help readability of the code instead of 
> getting the risk 
> of "feature not a bug" misread.. ;)..

Can we define omap3_errata on lines of omap3_features?

To the original question of why resue omap3_xyz name for omap4?
I guess very valid - unlike features, erratas may not be reused.
So it may make sense to have another omap4_errata as well. Any
duplications due to IP reuse can be handled on the omap3_errata
list (original IP where errata applies).

Deciding whether errata is feature or vice versa?
...ahem, pass.

Best regards,
Sanjeev

> 
> -- 
> Regards,
> Nishanth Menon
> --
> 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: FEATURES - is it good enough

[Aguirre, Sergio]

 

> -----Original Message-----
> From: Menon, Nishanth 
> Sent: Friday, November 20, 2009 1:24 PM
> To: Kevin Hilman
> Cc: Shilimkar, Santosh; Aguirre, Sergio; Pandita, Vikram; 
> linux-omap@vger.kernel.org
> Subject: Re: FEATURES - is it good enough
> 
> Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
> > "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> > 
> > [...]
> > 
> >>>> Probably not something ot be attached in this patch, but...
> >>>>
> >>>> I'm a bit curious about something:
> >>>>
> >>>> Why touching omap3_features in OMAP4?
> >>>>
> >>>> Isn't there a omap4_features?
> >>>>
> >>>> Or even better, an omap_features?
> >> This "is_feature" suppose to take care of Errata's also, is it?
> > 
> > "It's not a bug it's a feature." :)
> Bug. Santosh pointed out internally to h/w discussion which clearly 
> shows this as a h/w limitation. (thanks santosh)
> 
> > 
> >> This is errata more than a feature..... We better differentiate in
> >> this regard
> > 
> > I agree, I have a hard time calling this empty fifo read fault a
> > "feature."  We need a similar thing for errata.
> Agreed. This is a classic example why we need a common errata 
> handling 
> mechanism scalable across silicon variants on an IP basis. 
> two problems 
> in front of us:
> a) what do we want to do with 8250 workaround needed for omap3630 and 
> omap4? can we go ahead with features marking it clearly as a 
> "misuse of 
> features for the time being"

IMHO, That "for the time being" will stay forever if we don't do something now.

Most of the big problems are raised because someone says "ok, lets have this for
the time being". But that time never comes.

See that crazy CaMeL-Casing hanging around in /drivers/dsp/bridge/ for a while as
an example. When that will ever be fixed? I bet someone said some time:
"ok, lets fix it later" :-)

On the other hand. What's the big motivation to have this as a "feature"?

Who else than the serial driver cares about the "feature" awareness?

> b) a common silicon errata handling mechanism: Does anyone have 
> proposals for this? I can see it help in numerous places in our code 
> today and will help readability of the code instead of 
> getting the risk 
> of "feature not a bug" misread.. ;)..

My very personal opinion is that, good code commenting can make this very
clear.

But, the way it is right now, brings so much confusion (being a feature).

Regards,
Sergio

> 
> -- 
> Regards,
> Nishanth Menon
> 

RE: FEATURES - is it good enough

[Ramirez Luna, Omar]

>
>IMHO, That "for the time being" will stay forever if we don't do something now.
>
>Most of the big problems are raised because someone says "ok, lets have this for
>the time being". But that time never comes.
>
>See that crazy CaMeL-Casing hanging around in /drivers/dsp/bridge/ for a while as
>an example. When that will ever be fixed? I bet someone said some time:
>"ok, lets fix it later" :-)
>

IMHO read http://marc.info/?l=linux-omap&m=125729526105560&w=2

:P

- omar

Re: FEATURES - is it good enough

[Nishanth Menon]

Aguirre, Sergio had written, on 11/20/2009 01:43 PM, the following:
>  
> 
>> -----Original Message-----
>> From: Menon, Nishanth 
>> Sent: Friday, November 20, 2009 1:24 PM
>> To: Kevin Hilman
>> Cc: Shilimkar, Santosh; Aguirre, Sergio; Pandita, Vikram; 
>> linux-omap@vger.kernel.org
>> Subject: Re: FEATURES - is it good enough
>>
>> Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
>>> "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
>>>
>>> [...]
>>>
>>>>>> Probably not something ot be attached in this patch, but...
>>>>>>
>>>>>> I'm a bit curious about something:
>>>>>>
>>>>>> Why touching omap3_features in OMAP4?
>>>>>>
>>>>>> Isn't there a omap4_features?
>>>>>>
>>>>>> Or even better, an omap_features?
>>>> This "is_feature" suppose to take care of Errata's also, is it?
>>> "It's not a bug it's a feature." :)
>> Bug. Santosh pointed out internally to h/w discussion which clearly 
>> shows this as a h/w limitation. (thanks santosh)
>>
>>>> This is errata more than a feature..... We better differentiate in
>>>> this regard
>>> I agree, I have a hard time calling this empty fifo read fault a
>>> "feature."  We need a similar thing for errata.
>> Agreed. This is a classic example why we need a common errata 
>> handling 
>> mechanism scalable across silicon variants on an IP basis. 
>> two problems 
>> in front of us:
>> a) what do we want to do with 8250 workaround needed for omap3630 and 
>> omap4? can we go ahead with features marking it clearly as a 
>> "misuse of 
>> features for the time being"
> 
> IMHO, That "for the time being" will stay forever if we don't do something now.
> 
> Most of the big problems are raised because someone says "ok, lets have this for
> the time being". But that time never comes.
> 
> See that crazy CaMeL-Casing hanging around in /drivers/dsp/bridge/ for a while as
> an example. When that will ever be fixed? I bet someone said some time:
> "ok, lets fix it later" :-)
> 
> On the other hand. What's the big motivation to have this as a "feature"?
> 
> Who else than the serial driver cares about the "feature" awareness?

please see [1] and [2]. this wont be the first time I published 
something previously that got ignored and got re-discussed. note:

BTW, to be fair, DSPBridge already has plans to get fixed anyways..

Options I can think which were discussed:
a) introduce omap3_features omap3_errata: negative: wont read like if I 
use omap3_has_errata() for OMAP4 code.
b) introduce omap_features and omap_errata: negative: how do you link 
this to IP based usage across silicon (e.g. I2C).

I dont think these are scalable solutions though..

-- 
Regards,
Nishanth Menon
[1] http://marc.info/?l=linux-omap&m=125018632606920&w=2
[2] http://marc.info/?l=linux-omap&m=125319739327677&w=2

RE: FEATURES - is it good enough

[Aguirre, Sergio]

 

> -----Original Message-----
> From: Ramirez Luna, Omar 
> Sent: Friday, November 20, 2009 2:07 PM
> To: Aguirre, Sergio
> Cc: linux-omap@vger.kernel.org
> Subject: RE: FEATURES - is it good enough
> 
> >
> >IMHO, That "for the time being" will stay forever if we 
> don't do something now.
> >
> >Most of the big problems are raised because someone says 
> "ok, lets have this for
> >the time being". But that time never comes.
> >
> >See that crazy CaMeL-Casing hanging around in 
> /drivers/dsp/bridge/ for a while as
> >an example. When that will ever be fixed? I bet someone said 
> some time:
> >"ok, lets fix it later" :-)
> >
> 
> IMHO read http://marc.info/?l=linux-omap&m=125729526105560&w=2
> 
> :P

Damn... I didn't catch that one before. :-(

Apologies.

> 
> - omar
> 

RE: [PATCH v3 0/2] omap: serial: handle abort on empty rx fifo read

[Gadiyar, Anand]

Pandita, Vikram wrote:
> 
> introduce OMAP3_HAS_UART_NO_EMPTY_FIFO_READ feature
> 
> this feature is set for omap3630 and omap4 currently as 
> empty uart rx fifo read causes an abort
> 
> check on this feature on omap3630 and omap4 for now and extend for future
> vairants in future
> 
> Patch history:
> V1: initial implementation
> 	http://patchwork.kernel.org/patch/60785/
> 	http://patchwork.kernel.org/patch/60786/
> 
> V2: incorporate review comments from Alan Cox
> 	http://patchwork.kernel.org/patch/60785/
> 	to remove 8250.c file changes by override serial_in
> 	No 8250 driver change required now
> 
> V3: incorporate review comments
> 	khilman: make function override only for affected silicons
> 	nishant m: interoduce has_feature check, rather than cpu_is
> 	anand g: minor commit message change
> 
> Vikram Pandita (2):
>   omap: introduce uart_no_empty_fifo_read feature
>   omap: serial: fix non-empty uart fifo read abort
> 
>  arch/arm/mach-omap2/id.c              |    7 +++++++
>  arch/arm/mach-omap2/serial.c          |   21 +++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/cpu.h |    2 ++
>  3 files changed, 30 insertions(+), 0 deletions(-)
> 

Ping?

Tony,

These two patches are the only ones pending to make 3630 platforms boot
up nicely. Would you consider taking them?

- Anand

Re: FEATURES - is it good enough

[Alexander Shishkin]

On Fri, Nov 20, 2009 at 02:09:01 -0600, Nishanth Menon wrote:
> Aguirre, Sergio had written, on 11/20/2009 01:43 PM, the following:
> >
> >>-----Original Message-----
> >>From: Menon, Nishanth Sent: Friday, November 20, 2009 1:24 PM
> >>To: Kevin Hilman
> >>Cc: Shilimkar, Santosh; Aguirre, Sergio; Pandita, Vikram;
> >>linux-omap@vger.kernel.org
> >>Subject: Re: FEATURES - is it good enough
> >>
> >>Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
> >>>"Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> >>>
> >>>[...]
> >>>
> >>>>>>Probably not something ot be attached in this patch, but...
> >>>>>>
> >>>>>>I'm a bit curious about something:
> >>>>>>
> >>>>>>Why touching omap3_features in OMAP4?
> >>>>>>
> >>>>>>Isn't there a omap4_features?
> >>>>>>
> >>>>>>Or even better, an omap_features?
> >>>>This "is_feature" suppose to take care of Errata's also, is it?
> >>>"It's not a bug it's a feature." :)
> >>Bug. Santosh pointed out internally to h/w discussion which
> >>clearly shows this as a h/w limitation. (thanks santosh)
> >>
> >>>>This is errata more than a feature..... We better differentiate in
> >>>>this regard
> >>>I agree, I have a hard time calling this empty fifo read fault a
> >>>"feature."  We need a similar thing for errata.
> >>Agreed. This is a classic example why we need a common errata
> >>handling mechanism scalable across silicon variants on an IP
> >>basis. two problems in front of us:
> >>a) what do we want to do with 8250 workaround needed for
> >>omap3630 and omap4? can we go ahead with features marking it
> >>clearly as a "misuse of features for the time being"
> >
> >IMHO, That "for the time being" will stay forever if we don't do something now.
> >
> >Most of the big problems are raised because someone says "ok, lets have this for
> >the time being". But that time never comes.
> >
> >See that crazy CaMeL-Casing hanging around in /drivers/dsp/bridge/ for a while as
> >an example. When that will ever be fixed? I bet someone said some time:
> >"ok, lets fix it later" :-)
> >
> >On the other hand. What's the big motivation to have this as a "feature"?
> >
> >Who else than the serial driver cares about the "feature" awareness?
> 
> please see [1] and [2]. this wont be the first time I published
> something previously that got ignored and got re-discussed. note:

The [1] proposal sounds interesting to me, but it's not a very trivial matter.

> BTW, to be fair, DSPBridge already has plans to get fixed anyways..
> 
> Options I can think which were discussed:
> a) introduce omap3_features omap3_errata: negative: wont read like
> if I use omap3_has_errata() for OMAP4 code.
> b) introduce omap_features and omap_errata: negative: how do you
> link this to IP based usage across silicon (e.g. I2C).

How about omap_has_errata(module, errata)?
Or even something more generic?

Regards,
--
Alex

Re: [PATCH v3 0/2] omap: serial: handle abort on empty rx fifo read

[Alexander Shishkin]

On Tue, Dec 01, 2009 at 06:50:31 +0530, Gadiyar, Anand wrote:
> Pandita, Vikram wrote:
> > 
> > introduce OMAP3_HAS_UART_NO_EMPTY_FIFO_READ feature
> > 
> > this feature is set for omap3630 and omap4 currently as 
> > empty uart rx fifo read causes an abort
> > 
> > check on this feature on omap3630 and omap4 for now and extend for future
> > vairants in future
> > 
> > Patch history:
> > V1: initial implementation
> > 	http://patchwork.kernel.org/patch/60785/
> > 	http://patchwork.kernel.org/patch/60786/
> > 
> > V2: incorporate review comments from Alan Cox
> > 	http://patchwork.kernel.org/patch/60785/
> > 	to remove 8250.c file changes by override serial_in
> > 	No 8250 driver change required now
> > 
> > V3: incorporate review comments
> > 	khilman: make function override only for affected silicons
> > 	nishant m: interoduce has_feature check, rather than cpu_is
> > 	anand g: minor commit message change
> > 
> > Vikram Pandita (2):
> >   omap: introduce uart_no_empty_fifo_read feature
> >   omap: serial: fix non-empty uart fifo read abort
> > 
> >  arch/arm/mach-omap2/id.c              |    7 +++++++
> >  arch/arm/mach-omap2/serial.c          |   21 +++++++++++++++++++++
> >  arch/arm/plat-omap/include/plat/cpu.h |    2 ++
> >  3 files changed, 30 insertions(+), 0 deletions(-)
> > 
> 
> Ping?
> 
> Tony,
> 
> These two patches are the only ones pending to make 3630 platforms boot
> up nicely. Would you consider taking them?

Tested.
---cut---
[    0.000000] OMAP3630 ES1.0 (l2cache iva sgx neon isp uart_no_empty_fifo_read )
---cut---

Acked-by: Alexander Shishkin <virtuoso@slind.org>

Regards,
--
Alex

Re: FEATURES - is it good enough

[Menon, Nishanth]

Alexander Shishkin said the following on 12/01/2009 05:42 PM:
> On Fri, Nov 20, 2009 at 02:09:01 -0600, Nishanth Menon wrote:
>   
>> Aguirre, Sergio had written, on 11/20/2009 01:43 PM, the following:
>>     
>>>> -----Original Message-----
>>>> From: Menon, Nishanth Sent: Friday, November 20, 2009 1:24 PM
>>>> To: Kevin Hilman
>>>> Cc: Shilimkar, Santosh; Aguirre, Sergio; Pandita, Vikram;
>>>> linux-omap@vger.kernel.org
>>>> Subject: Re: FEATURES - is it good enough
>>>>
>>>> Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
>>>>         
>>>>> "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
>>>>>
>>>>> [...]
>>>>>
>>>>>           
>>>>>>>> Probably not something ot be attached in this patch, but...
>>>>>>>>
>>>>>>>> I'm a bit curious about something:
>>>>>>>>
>>>>>>>> Why touching omap3_features in OMAP4?
>>>>>>>>
>>>>>>>> Isn't there a omap4_features?
>>>>>>>>
>>>>>>>> Or even better, an omap_features?
>>>>>>>>                 
>>>>>> This "is_feature" suppose to take care of Errata's also, is it?
>>>>>>             
>>>>> "It's not a bug it's a feature." :)
>>>>>           
>>>> Bug. Santosh pointed out internally to h/w discussion which
>>>> clearly shows this as a h/w limitation. (thanks santosh)
>>>>
>>>>         
>>>>>> This is errata more than a feature..... We better differentiate in
>>>>>> this regard
>>>>>>             
>>>>> I agree, I have a hard time calling this empty fifo read fault a
>>>>> "feature."  We need a similar thing for errata.
>>>>>           
>>>> Agreed. This is a classic example why we need a common errata
>>>> handling mechanism scalable across silicon variants on an IP
>>>> basis. two problems in front of us:
>>>> a) what do we want to do with 8250 workaround needed for
>>>> omap3630 and omap4? can we go ahead with features marking it
>>>> clearly as a "misuse of features for the time being"
>>>>         
>>> IMHO, That "for the time being" will stay forever if we don't do something now.
>>>
>>> Most of the big problems are raised because someone says "ok, lets have this for
>>> the time being". But that time never comes.
>>>
>>> See that crazy CaMeL-Casing hanging around in /drivers/dsp/bridge/ for a while as
>>> an example. When that will ever be fixed? I bet someone said some time:
>>> "ok, lets fix it later" :-)
>>>
>>> On the other hand. What's the big motivation to have this as a "feature"?
>>>
>>> Who else than the serial driver cares about the "feature" awareness?
>>>       
>> please see [1] and [2]. this wont be the first time I published
>> something previously that got ignored and got re-discussed. note:
>>     
>
> The [1] proposal sounds interesting to me, but it's not a very trivial matter.
>
>   
>> BTW, to be fair, DSPBridge already has plans to get fixed anyways..
>>
>> Options I can think which were discussed:
>> a) introduce omap3_features omap3_errata: negative: wont read like
>> if I use omap3_has_errata() for OMAP4 code.
>> b) introduce omap_features and omap_errata: negative: how do you
>> link this to IP based usage across silicon (e.g. I2C).
>>     
>
> How about omap_has_errata(module, errata)?
> Or even something more generic?
>   
hmm.. just throwing more ideas up in the air:

Call method:
omap_ip_has_errata(u32 ip, u32 rev, u8 check_type,  u8 erratum)
where,
    ip = I2C, GPMC, MMC, etc..
    rev = revision number
    check_type = GT, LT, NE, EQ
    erratum = erratum type

omap_cpu_has_errata(u32 cpu_id, u32 rev, u8 check_type, u16 erratum)
where,
    cpu_id = 15xx, 16xx etc ( can this be a function pointer to
cpu_is..() functions? if so, how do we register this)
    rev = chip revision number
    check_type = GT, LT, NE, EQ
    erratum = erratum type
  
Registration/Initialization: ??

maybe this could be extended to features also..
Regards,
Nishanth Menon

Re: FEATURES - is it good enough

[Alexander Shishkin]

On Wed, Dec 02, 2009 at 08:06:16 +0200, Menon, Nishanth wrote:
> Alexander Shishkin said the following on 12/01/2009 05:42 PM:
> > On Fri, Nov 20, 2009 at 02:09:01 -0600, Nishanth Menon wrote:
> >   
> >> Aguirre, Sergio had written, on 11/20/2009 01:43 PM, the following:
> >>     
> >>>> -----Original Message-----
> >>>> From: Menon, Nishanth Sent: Friday, November 20, 2009 1:24 PM
> >>>> To: Kevin Hilman
> >>>> Cc: Shilimkar, Santosh; Aguirre, Sergio; Pandita, Vikram;
> >>>> linux-omap@vger.kernel.org
> >>>> Subject: Re: FEATURES - is it good enough
> >>>>
> >>>> Kevin Hilman had written, on 11/20/2009 12:35 PM, the following:
> >>>>         
> >>>>> "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>           
> >>>>>>>> Probably not something ot be attached in this patch, but...
> >>>>>>>>
> >>>>>>>> I'm a bit curious about something:
> >>>>>>>>
> >>>>>>>> Why touching omap3_features in OMAP4?
> >>>>>>>>
> >>>>>>>> Isn't there a omap4_features?
> >>>>>>>>
> >>>>>>>> Or even better, an omap_features?
> >>>>>>>>                 
> >>>>>> This "is_feature" suppose to take care of Errata's also, is it?
> >>>>>>             
> >>>>> "It's not a bug it's a feature." :)
> >>>>>           
> >>>> Bug. Santosh pointed out internally to h/w discussion which
> >>>> clearly shows this as a h/w limitation. (thanks santosh)
> >>>>
> >>>>         
> >>>>>> This is errata more than a feature..... We better differentiate in
> >>>>>> this regard
> >>>>>>             
> >>>>> I agree, I have a hard time calling this empty fifo read fault a
> >>>>> "feature."  We need a similar thing for errata.
> >>>>>           
> >>>> Agreed. This is a classic example why we need a common errata
> >>>> handling mechanism scalable across silicon variants on an IP
> >>>> basis. two problems in front of us:
> >>>> a) what do we want to do with 8250 workaround needed for
> >>>> omap3630 and omap4? can we go ahead with features marking it
> >>>> clearly as a "misuse of features for the time being"
> >>>>         
> >>> IMHO, That "for the time being" will stay forever if we don't do something now.
> >>>
> >>> Most of the big problems are raised because someone says "ok, lets have this for
> >>> the time being". But that time never comes.
> >>>
> >>> See that crazy CaMeL-Casing hanging around in /drivers/dsp/bridge/ for a while as
> >>> an example. When that will ever be fixed? I bet someone said some time:
> >>> "ok, lets fix it later" :-)
> >>>
> >>> On the other hand. What's the big motivation to have this as a "feature"?
> >>>
> >>> Who else than the serial driver cares about the "feature" awareness?
> >>>       
> >> please see [1] and [2]. this wont be the first time I published
> >> something previously that got ignored and got re-discussed. note:
> >>     
> >
> > The [1] proposal sounds interesting to me, but it's not a very trivial matter.
> >
> >   
> >> BTW, to be fair, DSPBridge already has plans to get fixed anyways..
> >>
> >> Options I can think which were discussed:
> >> a) introduce omap3_features omap3_errata: negative: wont read like
> >> if I use omap3_has_errata() for OMAP4 code.
> >> b) introduce omap_features and omap_errata: negative: how do you
> >> link this to IP based usage across silicon (e.g. I2C).
> >>     
> >
> > How about omap_has_errata(module, errata)?
> > Or even something more generic?
> >   
> hmm.. just throwing more ideas up in the air:
> 
> Call method:
> omap_ip_has_errata(u32 ip, u32 rev, u8 check_type,  u8 erratum)
> where,
>     ip = I2C, GPMC, MMC, etc..
>     rev = revision number
>     check_type = GT, LT, NE, EQ
>     erratum = erratum type
> 
> omap_cpu_has_errata(u32 cpu_id, u32 rev, u8 check_type, u16 erratum)
> where,
>     cpu_id = 15xx, 16xx etc ( can this be a function pointer to
> cpu_is..() functions? if so, how do we register this)
>     rev = chip revision number

Is there a use case when a caller would be interested in an erratum of a cpu
other than the one it is currently running on?
Otherwise it could be omap_this_cpu_has_errata(). And the same for IPs.

>     check_type = GT, LT, NE, EQ

Is this meant to compare cpu ids or revision numbers? I'm not sure I follow
the idea here.

>     erratum = erratum type
>   
> Registration/Initialization: ??

Perhaps, statically compiled in tables of per-cpu/rev erratas/features. This
has to be better than runtime detection and at least as good as making
assumptions based on cpu_is_omapXXXX().

> maybe this could be extended to features also..

Aren't features and erratas semantically the same thing? Maybe, the same
interface could be used.

Regards,
--
Alex