Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1627888 > unrolled thread

[PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support

Started byStefan Agner <stefan@agner.ch>
First post2017-04-21 03:10 +0200
Last post2017-04-21 20:50 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Stefan Agner <stefan@agner.ch> - 2017-04-21 03:10 +0200
    Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Marek Vasut <marek.vasut@gmail.com> - 2017-04-21 04:10 +0200
      Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Stefan Agner <stefan@agner.ch> - 2017-04-21 05:20 +0200
        Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Marek Vasut <marek.vasut@gmail.com> - 2017-04-21 15:10 +0200
          Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Stefan Agner <stefan@agner.ch> - 2017-04-21 19:50 +0200
            Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Han Xu <xhnjupt@gmail.com> - 2017-04-21 20:40 +0200
          Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Marek Vasut <marek.vasut@gmail.com> - 2017-04-21 20:30 +0200
          Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Stefan Agner <stefan@agner.ch> - 2017-04-21 20:50 +0200

#1627888 — [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support

FromStefan Agner <stefan@agner.ch>
Date2017-04-21 03:10 +0200
Subject[PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support
Message-ID<tyvfb-7P1-7@gated-at.bofh.it>
Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
clock architecture requiring only two clocks to be referenced.
The IP is slightly different compared to i.MX 6SoloX, but currently
none of this differences are in use so there is no detection needed
and the driver can reuse IS_MX6SX.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index c8bbf5da2ab8..4a45d37ddc80 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
 	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
 };
 
+static const char * const gpmi_clks_for_mx7d[] = {
+	"gpmi_io", "gpmi_bch_apb",
+};
+
+static const struct gpmi_devdata gpmi_devdata_imx7d = {
+	.type = IS_MX6SX,
+	.bch_max_ecc_strength = 62,
+	.max_chain_delay = 12,
+	.clks = gpmi_clks_for_mx7d,
+	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx7d),
+};
+
 static irqreturn_t bch_irq(int irq, void *cookie)
 {
 	struct gpmi_nand_data *this = cookie;
@@ -2071,6 +2083,9 @@ static const struct of_device_id gpmi_nand_id_table[] = {
 	}, {
 		.compatible = "fsl,imx6sx-gpmi-nand",
 		.data = &gpmi_devdata_imx6sx,
+	}, {
+		.compatible = "fsl,imx7d-gpmi-nand",
+		.data = &gpmi_devdata_imx7d,
 	}, {}
 };
 MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
-- 
2.12.2

[toc] | [next] | [standalone]

#1627912

FromMarek Vasut <marek.vasut@gmail.com>
Date2017-04-21 04:10 +0200
Message-ID<tywbf-8mP-1@gated-at.bofh.it>
In reply to#1627888
On 04/21/2017 03:07 AM, Stefan Agner wrote:
> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
> clock architecture requiring only two clocks to be referenced.
> The IP is slightly different compared to i.MX 6SoloX, but currently
> none of this differences are in use so there is no detection needed
> and the driver can reuse IS_MX6SX.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index c8bbf5da2ab8..4a45d37ddc80 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>  };
>  
> +static const char * const gpmi_clks_for_mx7d[] = {
> +	"gpmi_io", "gpmi_bch_apb",
> +};
> +
> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
> +	.type = IS_MX6SX,

Would it make sense to use IS_MX7 here already to prevent future surprises ?

> +	.bch_max_ecc_strength = 62,
> +	.max_chain_delay = 12,
> +	.clks = gpmi_clks_for_mx7d,
> +	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx7d),
> +};
> +
>  static irqreturn_t bch_irq(int irq, void *cookie)
>  {
>  	struct gpmi_nand_data *this = cookie;
> @@ -2071,6 +2083,9 @@ static const struct of_device_id gpmi_nand_id_table[] = {
>  	}, {
>  		.compatible = "fsl,imx6sx-gpmi-nand",
>  		.data = &gpmi_devdata_imx6sx,
> +	}, {
> +		.compatible = "fsl,imx7d-gpmi-nand",
> +		.data = &gpmi_devdata_imx7d,
>  	}, {}
>  };
>  MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
> 


-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1627931

FromStefan Agner <stefan@agner.ch>
Date2017-04-21 05:20 +0200
Message-ID<tyxgZ-CP-1@gated-at.bofh.it>
In reply to#1627912
On 2017-04-20 19:03, Marek Vasut wrote:
> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>> clock architecture requiring only two clocks to be referenced.
>> The IP is slightly different compared to i.MX 6SoloX, but currently
>> none of this differences are in use so there is no detection needed
>> and the driver can reuse IS_MX6SX.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> index c8bbf5da2ab8..4a45d37ddc80 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>  };
>>
>> +static const char * const gpmi_clks_for_mx7d[] = {
>> +	"gpmi_io", "gpmi_bch_apb",
>> +};
>> +
>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>> +	.type = IS_MX6SX,
> 
> Would it make sense to use IS_MX7 here already to prevent future surprises ?
> 

Yeah I was thinking we can do it once we have an actual reason to
distinguish.

But then, adding the type would only require 2-3 lines of change if I
add it to the GPMI_IS_MX6 macro...

--
Stefan

>> +	.bch_max_ecc_strength = 62,
>> +	.max_chain_delay = 12,
>> +	.clks = gpmi_clks_for_mx7d,
>> +	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx7d),
>> +};
>> +
>>  static irqreturn_t bch_irq(int irq, void *cookie)
>>  {
>>  	struct gpmi_nand_data *this = cookie;
>> @@ -2071,6 +2083,9 @@ static const struct of_device_id gpmi_nand_id_table[] = {
>>  	}, {
>>  		.compatible = "fsl,imx6sx-gpmi-nand",
>>  		.data = &gpmi_devdata_imx6sx,
>> +	}, {
>> +		.compatible = "fsl,imx7d-gpmi-nand",
>> +		.data = &gpmi_devdata_imx7d,
>>  	}, {}
>>  };
>>  MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
>>

[toc] | [prev] | [next] | [standalone]

#1628224

FromMarek Vasut <marek.vasut@gmail.com>
Date2017-04-21 15:10 +0200
Message-ID<tyGtY-6d0-7@gated-at.bofh.it>
In reply to#1627931
On 04/21/2017 05:15 AM, Stefan Agner wrote:
> On 2017-04-20 19:03, Marek Vasut wrote:
>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>> clock architecture requiring only two clocks to be referenced.
>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>> none of this differences are in use so there is no detection needed
>>> and the driver can reuse IS_MX6SX.
>>>
>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>> ---
>>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>  };
>>>
>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>> +	"gpmi_io", "gpmi_bch_apb",
>>> +};
>>> +
>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>> +	.type = IS_MX6SX,
>>
>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>
> 
> Yeah I was thinking we can do it once we have an actual reason to
> distinguish.

So what are the differences anyway ?

> But then, adding the type would only require 2-3 lines of change if I
> add it to the GPMI_IS_MX6 macro...

Then at least add a comment because using type = IMX6SX right under
gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.

-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1628396

FromStefan Agner <stefan@agner.ch>
Date2017-04-21 19:50 +0200
Message-ID<tyKQV-gp-7@gated-at.bofh.it>
In reply to#1628224
On 2017-04-21 10:22, Marek Vasut wrote:
> On 04/21/2017 06:19 PM, Stefan Agner wrote:
>> On 2017-04-21 06:08, Marek Vasut wrote:
>>> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>>>> On 2017-04-20 19:03, Marek Vasut wrote:
>>>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>>>> clock architecture requiring only two clocks to be referenced.
>>>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>>>> none of this differences are in use so there is no detection needed
>>>>>> and the driver can reuse IS_MX6SX.
>>>>>>
>>>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>>>> ---
>>>>>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>>>>  1 file changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>>>>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>>>>  };
>>>>>>
>>>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>>>> +	"gpmi_io", "gpmi_bch_apb",
>>>>>> +};
>>>>>> +
>>>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>>>> +	.type = IS_MX6SX,
>>>>>
>>>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>>>
>>>>
>>>> Yeah I was thinking we can do it once we have an actual reason to
>>>> distinguish.
>>>
>>> So what are the differences anyway ?
>>>
>>
>> I did not check the details, but Han's patchset (link in cover letter)
>> mentions:
>> "add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...
> 
> Oh, interesting.
> 
>>>> But then, adding the type would only require 2-3 lines of change if I
>>>> add it to the GPMI_IS_MX6 macro...
>>>
>>> Then at least add a comment because using type = IMX6SX right under
>>> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.
>>
>> FWIW, I mentioned it in the commit message.
>>
>> I think rather then adding a comment it is cleaner to just add IS_IMX7D
>> and add it to the GPMI_IS_MX6 macro. That does not need a comment since
>> it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
>> rather small change. Does that sound acceptable?
> 
> Sure, that's even better, thanks.
> 
> btw isn't there some single-core mx7 (mx7s ?) , maybe we should just go
> with mx7 (without the d suffix). I dunno if it has GPMI NAND though, so
> maybe mx7d is the right thing to do here ...
> 

There is a Solo version yes, and it has GPMI NAND too. However, almost
all i.MX 7 IPs have been named imx7d by NXP for some reason (including
compatible strings, see grep -r -e imx7 Documentation/), so I thought I
stay consistent here...

--
Stefan

>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> @@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
>>  };
>>
>>  static const struct gpmi_devdata gpmi_devdata_imx7d = {
>> -       .type = IS_MX6SX,
>> +       .type = IS_MX7D,
>>         .bch_max_ecc_strength = 62,
>>         .max_chain_delay = 12,
>>         .clks = gpmi_clks_for_mx7d,
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> index 2e584e18d980..f2cc13abc896 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> @@ -123,7 +123,8 @@ enum gpmi_type {
>>         IS_MX23,
>>         IS_MX28,
>>         IS_MX6Q,
>> -       IS_MX6SX
>> +       IS_MX6SX,
>> +       IS_MX7D,
>>  };
>>
>>  struct gpmi_devdata {
>> @@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
>>  #define GPMI_IS_MX28(x)                ((x)->devdata->type == IS_MX28)
>>  #define GPMI_IS_MX6Q(x)                ((x)->devdata->type == IS_MX6Q)
>>  #define GPMI_IS_MX6SX(x)       ((x)->devdata->type == IS_MX6SX)
>> +#define GPMI_IS_MX7D(x)                ((x)->devdata->type == IS_MX7D)
>>
>> -#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
>> +#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
>> \
>> +                                GPMI_IS_MX7D(x))
>>  #endif
>>
>> --
>> Stefan
>>

[toc] | [prev] | [next] | [standalone]

#1628474

FromHan Xu <xhnjupt@gmail.com>
Date2017-04-21 20:40 +0200
Message-ID<tyLDm-LM-63@gated-at.bofh.it>
In reply to#1628396
On Fri, Apr 21, 2017 at 12:38 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2017-04-21 10:22, Marek Vasut wrote:
>> On 04/21/2017 06:19 PM, Stefan Agner wrote:
>>> On 2017-04-21 06:08, Marek Vasut wrote:
>>>> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>>>>> On 2017-04-20 19:03, Marek Vasut wrote:
>>>>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>>>>> clock architecture requiring only two clocks to be referenced.
>>>>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>>>>> none of this differences are in use so there is no detection needed
>>>>>>> and the driver can reuse IS_MX6SX.
>>>>>>>
>>>>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>>>>> ---
>>>>>>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>>>>>  1 file changed, 15 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>>>>>          .clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>>>>>  };
>>>>>>>
>>>>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>>>>> +        "gpmi_io", "gpmi_bch_apb",
>>>>>>> +};
>>>>>>> +
>>>>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>>>>> +        .type = IS_MX6SX,
>>>>>>
>>>>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>>>>
>>>>>
>>>>> Yeah I was thinking we can do it once we have an actual reason to
>>>>> distinguish.
>>>>
>>>> So what are the differences anyway ?
>>>>
>>>
>>> I did not check the details, but Han's patchset (link in cover letter)
>>> mentions:
>>> "add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...
>>
>> Oh, interesting.
>>
>>>>> But then, adding the type would only require 2-3 lines of change if I
>>>>> add it to the GPMI_IS_MX6 macro...
>>>>
>>>> Then at least add a comment because using type = IMX6SX right under
>>>> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.
>>>
>>> FWIW, I mentioned it in the commit message.
>>>
>>> I think rather then adding a comment it is cleaner to just add IS_IMX7D
>>> and add it to the GPMI_IS_MX6 macro. That does not need a comment since
>>> it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
>>> rather small change. Does that sound acceptable?
>>
>> Sure, that's even better, thanks.
>>
>> btw isn't there some single-core mx7 (mx7s ?) , maybe we should just go
>> with mx7 (without the d suffix). I dunno if it has GPMI NAND though, so
>> maybe mx7d is the right thing to do here ...
>>
>
> There is a Solo version yes, and it has GPMI NAND too. However, almost
> all i.MX 7 IPs have been named imx7d by NXP for some reason (including
> compatible strings, see grep -r -e imx7 Documentation/), so I thought I
> stay consistent here...

Hi Guys,

Yes, there should be a i.MX7 Solo version with one core fused out. IMO, can
we use QUIRK to distinguish them rather than SoC name. I know I also sent
some patch set with SoC Name but I prefer to use QUIRK now.

>
> --
> Stefan
>
>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> @@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
>>>  };
>>>
>>>  static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>> -       .type = IS_MX6SX,
>>> +       .type = IS_MX7D,
>>>         .bch_max_ecc_strength = 62,
>>>         .max_chain_delay = 12,
>>>         .clks = gpmi_clks_for_mx7d,
>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>>> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>>> index 2e584e18d980..f2cc13abc896 100644
>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>>> @@ -123,7 +123,8 @@ enum gpmi_type {
>>>         IS_MX23,
>>>         IS_MX28,
>>>         IS_MX6Q,
>>> -       IS_MX6SX
>>> +       IS_MX6SX,
>>> +       IS_MX7D,
>>>  };
>>>
>>>  struct gpmi_devdata {
>>> @@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
>>>  #define GPMI_IS_MX28(x)                ((x)->devdata->type == IS_MX28)
>>>  #define GPMI_IS_MX6Q(x)                ((x)->devdata->type == IS_MX6Q)
>>>  #define GPMI_IS_MX6SX(x)       ((x)->devdata->type == IS_MX6SX)
>>> +#define GPMI_IS_MX7D(x)                ((x)->devdata->type == IS_MX7D)
>>>
>>> -#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
>>> +#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
>>> \
>>> +                                GPMI_IS_MX7D(x))
>>>  #endif
>>>
>>> --
>>> Stefan
>>>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

[toc] | [prev] | [next] | [standalone]

#1628434

FromMarek Vasut <marek.vasut@gmail.com>
Date2017-04-21 20:30 +0200
Message-ID<tyKQV-gp-11@gated-at.bofh.it>
In reply to#1628224
On 04/21/2017 06:19 PM, Stefan Agner wrote:
> On 2017-04-21 06:08, Marek Vasut wrote:
>> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>>> On 2017-04-20 19:03, Marek Vasut wrote:
>>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>>> clock architecture requiring only two clocks to be referenced.
>>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>>> none of this differences are in use so there is no detection needed
>>>>> and the driver can reuse IS_MX6SX.
>>>>>
>>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>>> ---
>>>>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>>>  1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>>>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>>>  };
>>>>>
>>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>>> +	"gpmi_io", "gpmi_bch_apb",
>>>>> +};
>>>>> +
>>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>>> +	.type = IS_MX6SX,
>>>>
>>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>>
>>>
>>> Yeah I was thinking we can do it once we have an actual reason to
>>> distinguish.
>>
>> So what are the differences anyway ?
>>
> 
> I did not check the details, but Han's patchset (link in cover letter)
> mentions:
> "add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...

Oh, interesting.

>>> But then, adding the type would only require 2-3 lines of change if I
>>> add it to the GPMI_IS_MX6 macro...
>>
>> Then at least add a comment because using type = IMX6SX right under
>> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.
> 
> FWIW, I mentioned it in the commit message.
> 
> I think rather then adding a comment it is cleaner to just add IS_IMX7D
> and add it to the GPMI_IS_MX6 macro. That does not need a comment since
> it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
> rather small change. Does that sound acceptable?

Sure, that's even better, thanks.

btw isn't there some single-core mx7 (mx7s ?) , maybe we should just go
with mx7 (without the d suffix). I dunno if it has GPMI NAND though, so
maybe mx7d is the right thing to do here ...

> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
>  };
> 
>  static const struct gpmi_devdata gpmi_devdata_imx7d = {
> -       .type = IS_MX6SX,
> +       .type = IS_MX7D,
>         .bch_max_ecc_strength = 62,
>         .max_chain_delay = 12,
>         .clks = gpmi_clks_for_mx7d,
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> index 2e584e18d980..f2cc13abc896 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> @@ -123,7 +123,8 @@ enum gpmi_type {
>         IS_MX23,
>         IS_MX28,
>         IS_MX6Q,
> -       IS_MX6SX
> +       IS_MX6SX,
> +       IS_MX7D,
>  };
> 
>  struct gpmi_devdata {
> @@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
>  #define GPMI_IS_MX28(x)                ((x)->devdata->type == IS_MX28)
>  #define GPMI_IS_MX6Q(x)                ((x)->devdata->type == IS_MX6Q)
>  #define GPMI_IS_MX6SX(x)       ((x)->devdata->type == IS_MX6SX)
> +#define GPMI_IS_MX7D(x)                ((x)->devdata->type == IS_MX7D)
> 
> -#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
> +#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
> \
> +                                GPMI_IS_MX7D(x))
>  #endif
> 
> --
> Stefan
> 


-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1628479

FromStefan Agner <stefan@agner.ch>
Date2017-04-21 20:50 +0200
Message-ID<tyKQV-gp-9@gated-at.bofh.it>
In reply to#1628224
On 2017-04-21 06:08, Marek Vasut wrote:
> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>> On 2017-04-20 19:03, Marek Vasut wrote:
>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>> clock architecture requiring only two clocks to be referenced.
>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>> none of this differences are in use so there is no detection needed
>>>> and the driver can reuse IS_MX6SX.
>>>>
>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>> ---
>>>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>>  1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>>  	.clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>>  };
>>>>
>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>> +	"gpmi_io", "gpmi_bch_apb",
>>>> +};
>>>> +
>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>> +	.type = IS_MX6SX,
>>>
>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>
>>
>> Yeah I was thinking we can do it once we have an actual reason to
>> distinguish.
> 
> So what are the differences anyway ?
> 

I did not check the details, but Han's patchset (link in cover letter)
mentions:
"add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...

>> But then, adding the type would only require 2-3 lines of change if I
>> add it to the GPMI_IS_MX6 macro...
> 
> Then at least add a comment because using type = IMX6SX right under
> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.

FWIW, I mentioned it in the commit message.

I think rather then adding a comment it is cleaner to just add IS_IMX7D
and add it to the GPMI_IS_MX6 macro. That does not need a comment since
it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
rather small change. Does that sound acceptable?

--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
 };

 static const struct gpmi_devdata gpmi_devdata_imx7d = {
-       .type = IS_MX6SX,
+       .type = IS_MX7D,
        .bch_max_ecc_strength = 62,
        .max_chain_delay = 12,
        .clks = gpmi_clks_for_mx7d,
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 2e584e18d980..f2cc13abc896 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -123,7 +123,8 @@ enum gpmi_type {
        IS_MX23,
        IS_MX28,
        IS_MX6Q,
-       IS_MX6SX
+       IS_MX6SX,
+       IS_MX7D,
 };

 struct gpmi_devdata {
@@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
 #define GPMI_IS_MX28(x)                ((x)->devdata->type == IS_MX28)
 #define GPMI_IS_MX6Q(x)                ((x)->devdata->type == IS_MX6Q)
 #define GPMI_IS_MX6SX(x)       ((x)->devdata->type == IS_MX6SX)
+#define GPMI_IS_MX7D(x)                ((x)->devdata->type == IS_MX7D)

-#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
+#define GPMI_IS_MX6(x)         (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
\
+                                GPMI_IS_MX7D(x))
 #endif

--
Stefan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web