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@xxxxxxxx> >>>> --- >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html