[PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Aaron Sierra]

Freescale's QorIQ T Series processors support 8 IFC chip selects
within a memory map backward compatible with previous P Series
processors which supported only 4 chip selects.

Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
---
 include/linux/fsl_ifc.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 84d60cb..62762ff 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -29,7 +29,7 @@
 #include <linux/of_platform.h>
 #include <linux/interrupt.h>
 
-#define FSL_IFC_BANK_COUNT 4
+#define FSL_IFC_BANK_COUNT 8
 
 /*
  * CSPR - Chip Select Property Register
@@ -776,23 +776,23 @@ struct fsl_ifc_regs {
 		__be32 cspr;
 		u32 res2;
 	} cspr_cs[FSL_IFC_BANK_COUNT];
-	u32 res3[0x19];
+	u32 res3[0xd];
 	struct {
 		__be32 amask;
 		u32 res4[0x2];
 	} amask_cs[FSL_IFC_BANK_COUNT];
-	u32 res5[0x18];
+	u32 res5[0xc];
 	struct {
 		__be32 csor;
 		__be32 csor_ext;
 		u32 res6;
 	} csor_cs[FSL_IFC_BANK_COUNT];
-	u32 res7[0x18];
+	u32 res7[0xc];
 	struct {
 		__be32 ftim[4];
 		u32 res8[0x8];
 	} ftim_cs[FSL_IFC_BANK_COUNT];
-	u32 res9[0x60];
+	u32 res9[0x30];
 	__be32 rb_stat;
 	u32 res10[0x2];
 	__be32 ifc_gcr;
-- 
1.9.1

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Scott Wood]

On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
> Freescale's QorIQ T Series processors support 8 IFC chip selects
> within a memory map backward compatible with previous P Series
> processors which supported only 4 chip selects.
> 
> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> ---
>  include/linux/fsl_ifc.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> index 84d60cb..62762ff 100644
> --- a/include/linux/fsl_ifc.h
> +++ b/include/linux/fsl_ifc.h
> @@ -29,7 +29,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/interrupt.h>
>  
> -#define FSL_IFC_BANK_COUNT 4
> +#define FSL_IFC_BANK_COUNT 8

First please modify fsl_ifc_nand.c to limit itself to the number of
banks it dynamically determines are present based on the IFC version.

-Scott

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Prabhakar Kushwaha]

On 8/20/2014 5:38 AM, Scott Wood wrote:
> On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
>> Freescale's QorIQ T Series processors support 8 IFC chip selects
>> within a memory map backward compatible with previous P Series
>> processors which supported only 4 chip selects.
>>
>> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
>> ---
>>   include/linux/fsl_ifc.h | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
>> index 84d60cb..62762ff 100644
>> --- a/include/linux/fsl_ifc.h
>> +++ b/include/linux/fsl_ifc.h
>> @@ -29,7 +29,7 @@
>>   #include <linux/of_platform.h>
>>   #include <linux/interrupt.h>
>>   
>> -#define FSL_IFC_BANK_COUNT 4
>> +#define FSL_IFC_BANK_COUNT 8
> First please modify fsl_ifc_nand.c to limit itself to the number of
> banks it dynamically determines are present based on the IFC version.
>
>

Number of available bank/chip select are defined by SoC and it is 
independent of SoC.
It should be fix in following way

Option 1:
u-boot:  fix device tree with number of available chip select. It may 
require IFC binding change
Linux: Read device tree and determine the Chip Selects

or

Option 2:
Make it static because any way IFC NAND driver polls to 
FSL_IFC_BANK_COUNT to know NAND flash chip select. This patch is doing same.

Regards,
Prabhakar

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Aaron Sierra]

> >> -#define FSL_IFC_BANK_COUNT 4
> >> +#define FSL_IFC_BANK_COUNT 8
> > First please modify fsl_ifc_nand.c to limit itself to the number of
> > banks it dynamically determines are present based on the IFC version.
> >
> >
> 
> Number of available bank/chip select are defined by SoC and it is
> independent of SoC.

You mean that there is no way to tell from an IFC-specific register how
many chip selects are valid for a given SoC, correct?

> It should be fix in following way
> 
> Option 1:
> u-boot:  fix device tree with number of available chip select. It may
> require IFC binding change
> Linux: Read device tree and determine the Chip Selects
> 
> or
> 
> Option 2:
> Make it static because any way IFC NAND driver polls to
> FSL_IFC_BANK_COUNT to know NAND flash chip select. This patch is doing same.

My patch is based on the assumption that it is safe to access the addresses of
the four nonexistent (unconnected?) chip selects in devices with fewer than
eight chip selects.

Also, the full FSL_IFC_BANK_COUNT range of chip selects should only be probed
when no match is not found for a chip select's DTS-defined address base within
any IFC bank. The typical case would be for the device tree to properly define
the address space prepared by the bootloader, which would result in only banks
present in the SoC being probed.

I have tested this patch on P1010 and T1042 processors, which feature four and
eight chip selects respectively with no apparent ill effects.

Our P1010 product has NAND attached to chip selects 0 and 1:

    ranges = <0x0 0x0 0x0 0xef800000 0x0010000
              0x1 0x0 0x0 0xef840000 0x0010000>;

I mangled chip-select 0 so that the DTS-defined base would not match the
address programmed by firmware, so that all eight chip selects would be
scanned on this four chip-select part:

    ranges = <0x0 0x0 0x0 0xefc00000 0x0010000
              0x1 0x0 0x0 0xef840000 0x0010000>;

This resulted in the following kernel message:

fsl,ifc-nand efc00000.nand0: fsl_ifc_nand_probe: address did not match any chip selects

The NAND device at chip select 1 was properly detected.

> Regards,
> Prabhakar

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Scott Wood]

On Wed, 2014-08-20 at 09:05 +0530, Prabhakar Kushwaha wrote:
> On 8/20/2014 5:38 AM, Scott Wood wrote:
> > On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
> >> Freescale's QorIQ T Series processors support 8 IFC chip selects
> >> within a memory map backward compatible with previous P Series
> >> processors which supported only 4 chip selects.
> >>
> >> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> >> ---
> >>   include/linux/fsl_ifc.h | 10 +++++-----
> >>   1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> >> index 84d60cb..62762ff 100644
> >> --- a/include/linux/fsl_ifc.h
> >> +++ b/include/linux/fsl_ifc.h
> >> @@ -29,7 +29,7 @@
> >>   #include <linux/of_platform.h>
> >>   #include <linux/interrupt.h>
> >>   
> >> -#define FSL_IFC_BANK_COUNT 4
> >> +#define FSL_IFC_BANK_COUNT 8
> > First please modify fsl_ifc_nand.c to limit itself to the number of
> > banks it dynamically determines are present based on the IFC version.
> >
> >
> 
> Number of available bank/chip select are defined by SoC and it is 
> independent of SoC.

Do you mean defined by the SoC and independent of the IFC version?

> It should be fix in following way
> 
> Option 1:
> u-boot:  fix device tree with number of available chip select. It may 
> require IFC binding change
> Linux: Read device tree and determine the Chip Selects

If we do this then it will need to be an optional property that defaults
to the current assumption being made (4).

In the future we really ought to check whether there are integration
parameters when coming up with the initial binding for a hardware
block...

> Option 2:
> Make it static because any way IFC NAND driver polls to 
> FSL_IFC_BANK_COUNT to know NAND flash chip select. This patch is doing same.

I don't understand what you're saying here.  The driver does not know at
compile time how many there are.  What this patch does is assume it's OK
to access non-existent registers in the rare case that there's no match
in the registers that exist.

Aaron tested this on P1010 and it seemed to work, though I'm not
generally fond of relying on such things.

-Scott

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Prabhakar Kushwaha]

Sorry Scott for late reply,

Please find my reply in-lined


On 8/21/2014 4:51 AM, Scott Wood wrote:
> On Wed, 2014-08-20 at 09:05 +0530, Prabhakar Kushwaha wrote:
>> On 8/20/2014 5:38 AM, Scott Wood wrote:
>>> On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
>>>> Freescale's QorIQ T Series processors support 8 IFC chip selects
>>>> within a memory map backward compatible with previous P Series
>>>> processors which supported only 4 chip selects.
>>>>
>>>> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
>>>> ---
>>>>    include/linux/fsl_ifc.h | 10 +++++-----
>>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
>>>> index 84d60cb..62762ff 100644
>>>> --- a/include/linux/fsl_ifc.h
>>>> +++ b/include/linux/fsl_ifc.h
>>>> @@ -29,7 +29,7 @@
>>>>    #include <linux/of_platform.h>
>>>>    #include <linux/interrupt.h>
>>>>    
>>>> -#define FSL_IFC_BANK_COUNT 4
>>>> +#define FSL_IFC_BANK_COUNT 8
>>> First please modify fsl_ifc_nand.c to limit itself to the number of
>>> banks it dynamically determines are present based on the IFC version.
>>>
>>>
>> Number of available bank/chip select are defined by SoC and it is
>> independent of SoC.
> Do you mean defined by the SoC and independent of the IFC version?

IFC v 1.0.0 supports 4 Chip Select.
IFC v 1.1.0 onwards, IFC supports 8 chip select.

But SoC finally defines number of chip select coming out of SoC. Like 
LS1021A with IFC ver 1.4.0 have only 7 Chip Select.

>> It should be fix in following way
>>
>> Option 1:
>> u-boot:  fix device tree with number of available chip select. It may
>> require IFC binding change
>> Linux: Read device tree and determine the Chip Selects
> If we do this then it will need to be an optional property that defaults
> to the current assumption being made (4).

Yes, I agree with you.

> In the future we really ought to check whether there are integration
> parameters when coming up with the initial binding for a hardware
> block...

True.


>> Option 2:
>> Make it static because any way IFC NAND driver polls to
>> FSL_IFC_BANK_COUNT to know NAND flash chip select. This patch is doing same.
> I don't understand what you're saying here.  The driver does not know at
> compile time how many there are.  What this patch does is assume it's OK
> to access non-existent registers in the rare case that there's no match
> in the registers that exist.

in case of P1010, If we run loop for 8 times,
      we are accessing reserved memory, that's why it may work. In Ideal 
scenario, we should not access the reserved memory.


> Aaron tested this on P1010 and it seemed to work, though I'm not
> generally fond of relying on such things.
>
yes, I agree.

  If I conclude  ==>   We should go with option 1.  Am I correct?


Regards,
Prabhakar

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Scott Wood]

On Fri, 2014-08-22 at 20:07 +0530, Prabhakar Kushwaha wrote:
> Sorry Scott for late reply,
> 
> Please find my reply in-lined
> 
> 
> On 8/21/2014 4:51 AM, Scott Wood wrote:
> > On Wed, 2014-08-20 at 09:05 +0530, Prabhakar Kushwaha wrote:
> >> On 8/20/2014 5:38 AM, Scott Wood wrote:
> >>> On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
> >>>> Freescale's QorIQ T Series processors support 8 IFC chip selects
> >>>> within a memory map backward compatible with previous P Series
> >>>> processors which supported only 4 chip selects.
> >>>>
> >>>> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> >>>> ---
> >>>>    include/linux/fsl_ifc.h | 10 +++++-----
> >>>>    1 file changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> >>>> index 84d60cb..62762ff 100644
> >>>> --- a/include/linux/fsl_ifc.h
> >>>> +++ b/include/linux/fsl_ifc.h
> >>>> @@ -29,7 +29,7 @@
> >>>>    #include <linux/of_platform.h>
> >>>>    #include <linux/interrupt.h>
> >>>>    
> >>>> -#define FSL_IFC_BANK_COUNT 4
> >>>> +#define FSL_IFC_BANK_COUNT 8
> >>> First please modify fsl_ifc_nand.c to limit itself to the number of
> >>> banks it dynamically determines are present based on the IFC version.
> >>>
> >>>
> >> Number of available bank/chip select are defined by SoC and it is
> >> independent of SoC.
> > Do you mean defined by the SoC and independent of the IFC version?
> 
> IFC v 1.0.0 supports 4 Chip Select.
> IFC v 1.1.0 onwards, IFC supports 8 chip select.
> 
> But SoC finally defines number of chip select coming out of SoC. Like 
> LS1021A with IFC ver 1.4.0 have only 7 Chip Select.

What matters here is whether the registers are implemented, not whether
a chip select is pinned out -- so use the IFC version.

-Scott

Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects

[Prabhakar Kushwaha]

On 8/22/2014 11:21 PM, Scott Wood wrote:
> On Fri, 2014-08-22 at 20:07 +0530, Prabhakar Kushwaha wrote:
>> Sorry Scott for late reply,
>>
>> Please find my reply in-lined
>>
>>
>> On 8/21/2014 4:51 AM, Scott Wood wrote:
>>> On Wed, 2014-08-20 at 09:05 +0530, Prabhakar Kushwaha wrote:
>>>> On 8/20/2014 5:38 AM, Scott Wood wrote:
>>>>> On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
>>>>>> Freescale's QorIQ T Series processors support 8 IFC chip selects
>>>>>> within a memory map backward compatible with previous P Series
>>>>>> processors which supported only 4 chip selects.
>>>>>>
>>>>>> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
>>>>>> ---
>>>>>>     include/linux/fsl_ifc.h | 10 +++++-----
>>>>>>     1 file changed, 5 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
>>>>>> index 84d60cb..62762ff 100644
>>>>>> --- a/include/linux/fsl_ifc.h
>>>>>> +++ b/include/linux/fsl_ifc.h
>>>>>> @@ -29,7 +29,7 @@
>>>>>>     #include <linux/of_platform.h>
>>>>>>     #include <linux/interrupt.h>
>>>>>>     
>>>>>> -#define FSL_IFC_BANK_COUNT 4
>>>>>> +#define FSL_IFC_BANK_COUNT 8
>>>>> First please modify fsl_ifc_nand.c to limit itself to the number of
>>>>> banks it dynamically determines are present based on the IFC version.
>>>>>
>>>>>
>>>> Number of available bank/chip select are defined by SoC and it is
>>>> independent of SoC.
>>> Do you mean defined by the SoC and independent of the IFC version?
>> IFC v 1.0.0 supports 4 Chip Select.
>> IFC v 1.1.0 onwards, IFC supports 8 chip select.
>>
>> But SoC finally defines number of chip select coming out of SoC. Like
>> LS1021A with IFC ver 1.4.0 have only 7 Chip Select.
> What matters here is whether the registers are implemented, not whether
> a chip select is pinned out -- so use the IFC version.

I checked with IFC IP team " There is no side-effect of reading CS-8  
register if only 7 CS exposed by SoC"

IFC version can be used.

Regards,
Prabhakar