| Message ID | 1328960705-18699-7-git-send-email-hechtb@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
Hi Bastian, Thanks for the patch. On Saturday 11 February 2012 12:45:04 Bastian Hecht wrote: > Add a register used in new FLCTL hardware and a feature flag for it. > > Signed-off-by: Bastian Hecht <hechtb@gmail.com> > --- > changelog: the write to the register has been moved due to patch 5. > > drivers/mtd/nand/sh_flctl.c | 3 +++ > include/linux/mtd/sh_flctl.h | 12 ++++++++++++ > 2 files changed, 15 insertions(+), 0 deletions(-) > > diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c > index 1af41fd..40dda26 100644 > --- a/drivers/mtd/nand/sh_flctl.c > +++ b/drivers/mtd/nand/sh_flctl.c > @@ -688,6 +688,8 @@ static void flctl_select_chip(struct mtd_info *mtd, int > chipnr) break; > case 0: > writel(flctl->flcmncr_val | CE0_ENABLE, FLCMNCR(flctl)); > + if (flctl->holden) > + writel(HOLDEN, FLHOLDCR(flctl)); Can't this be done at probe time (maybe in flctl_chip_init_tail()) ? You could then get rid of the flctl->holden field and use platform data directly. > break; > default: > BUG(); > @@ -845,6 +847,7 @@ static int __devinit flctl_probe(struct platform_device > *pdev) flctl->pdev = pdev; > flctl->flcmncr_val = pdata->flcmncr_val; > flctl->hwecc = pdata->has_hwecc; > + flctl->holden = pdata->use_holden; > > nand->options = NAND_NO_AUTOINCR; > > diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h > index 107fd8a..6046443 100644 > --- a/include/linux/mtd/sh_flctl.h > +++ b/include/linux/mtd/sh_flctl.h > @@ -38,6 +38,7 @@ > #define FLDTFIFO(f) (f->reg + 0x24) > #define FLECFIFO(f) (f->reg + 0x28) > #define FLTRCR(f) (f->reg + 0x2C) > +#define FLHOLDCR(f) (f->reg + 0x38) > #define FL4ECCRESULT0(f) (f->reg + 0x80) > #define FL4ECCRESULT1(f) (f->reg + 0x84) > #define FL4ECCRESULT2(f) (f->reg + 0x88) > @@ -109,6 +110,15 @@ > #define TRSTRT (0x1 << 0) /* translation start */ > #define TREND (0x1 << 1) /* translation end */ > > +/* > + * FLHOLDCR control bits > + * > + * HOLDEN: Bus Occupancy Enable (inverted) > + * Enable this bit when the external bus might be used in between > transfers. + * If not set and the bus gets used by other modules, a > deadlock occurs. + */ > +#define HOLDEN (0x1 << 0) > + > /* FL4ECCCR control bits */ > #define _4ECCFA (0x1 << 2) /* 4 symbols correct fault */ > #define _4ECCEND (0x1 << 1) /* 4 symbols end */ > @@ -138,6 +148,7 @@ struct sh_flctl { > > unsigned page_size:1; /* NAND page size (0 = 512, 1 = 2048) */ > unsigned hwecc:1; /* Hardware ECC (0 = disabled, 1 = enabled) */ > + unsigned holden:1; /* Hardware has FLHOLDCR and HOLDEN is set */ > }; > > struct sh_flctl_platform_data { > @@ -146,6 +157,7 @@ struct sh_flctl_platform_data { > unsigned long flcmncr_val; > > unsigned has_hwecc:1; > + unsigned use_holden:1; > }; > > static inline struct sh_flctl *mtd_to_flctl(struct mtd_info *mtdinfo)
Hello Laurent, 2012/2/18 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Bastian, > > Thanks for the patch. > > On Saturday 11 February 2012 12:45:04 Bastian Hecht wrote: >> Add a register used in new FLCTL hardware and a feature flag for it. >> >> Signed-off-by: Bastian Hecht <hechtb@gmail.com> >> --- >> changelog: the write to the register has been moved due to patch 5. >> >> drivers/mtd/nand/sh_flctl.c | 3 +++ >> include/linux/mtd/sh_flctl.h | 12 ++++++++++++ >> 2 files changed, 15 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c >> index 1af41fd..40dda26 100644 >> --- a/drivers/mtd/nand/sh_flctl.c >> +++ b/drivers/mtd/nand/sh_flctl.c >> @@ -688,6 +688,8 @@ static void flctl_select_chip(struct mtd_info *mtd, int >> chipnr) break; >> case 0: >> writel(flctl->flcmncr_val | CE0_ENABLE, FLCMNCR(flctl)); >> + if (flctl->holden) >> + writel(HOLDEN, FLHOLDCR(flctl)); > > Can't this be done at probe time (maybe in flctl_chip_init_tail()) ? You could > then get rid of the flctl->holden field and use platform data directly. I need this information for runtime pm. I reworked this patch series to be without rtpm, but from the next patch on (that's only waiting to be posted), I'll want to have it like this. In fact this is a design question: I can either have a resume/runtime_resume callback whose only job is to set FLHOLDCR. or I integrate this into select_chip. The first variation saves 1 register write in case of no rtpm activated - the second variation makes the code a bit slimmer. What do you think? 1. Leave it like this and save the callbacks in the future 2. Rework it to be set at probe time() and postpone the design decision? 3. Rework it to be set at probe time() and implement callbacks in the upcoming patches? >> break; >> default: >> BUG(); >> @@ -845,6 +847,7 @@ static int __devinit flctl_probe(struct platform_device >> *pdev) flctl->pdev = pdev; >> flctl->flcmncr_val = pdata->flcmncr_val; >> flctl->hwecc = pdata->has_hwecc; >> + flctl->holden = pdata->use_holden; >> >> nand->options = NAND_NO_AUTOINCR; >> >> diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h >> index 107fd8a..6046443 100644 >> --- a/include/linux/mtd/sh_flctl.h >> +++ b/include/linux/mtd/sh_flctl.h >> @@ -38,6 +38,7 @@ >> #define FLDTFIFO(f) (f->reg + 0x24) >> #define FLECFIFO(f) (f->reg + 0x28) >> #define FLTRCR(f) (f->reg + 0x2C) >> +#define FLHOLDCR(f) (f->reg + 0x38) >> #define FL4ECCRESULT0(f) (f->reg + 0x80) >> #define FL4ECCRESULT1(f) (f->reg + 0x84) >> #define FL4ECCRESULT2(f) (f->reg + 0x88) >> @@ -109,6 +110,15 @@ >> #define TRSTRT (0x1 << 0) /* translation start */ >> #define TREND (0x1 << 1) /* translation end */ >> >> +/* >> + * FLHOLDCR control bits >> + * >> + * HOLDEN: Bus Occupancy Enable (inverted) >> + * Enable this bit when the external bus might be used in between >> transfers. + * If not set and the bus gets used by other modules, a >> deadlock occurs. + */ >> +#define HOLDEN (0x1 << 0) >> + >> /* FL4ECCCR control bits */ >> #define _4ECCFA (0x1 << 2) /* 4 symbols correct fault */ >> #define _4ECCEND (0x1 << 1) /* 4 symbols end */ >> @@ -138,6 +148,7 @@ struct sh_flctl { >> >> unsigned page_size:1; /* NAND page size (0 = 512, 1 = 2048) */ >> unsigned hwecc:1; /* Hardware ECC (0 = disabled, 1 = enabled) */ >> + unsigned holden:1; /* Hardware has FLHOLDCR and HOLDEN is set */ >> }; >> >> struct sh_flctl_platform_data { >> @@ -146,6 +157,7 @@ struct sh_flctl_platform_data { >> unsigned long flcmncr_val; >> >> unsigned has_hwecc:1; >> + unsigned use_holden:1; >> }; >> >> static inline struct sh_flctl *mtd_to_flctl(struct mtd_info *mtdinfo) > > -- > Regards, > > Laurent Pinchart thanks for all the reviews! Bastian
diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c index 1af41fd..40dda26 100644 --- a/drivers/mtd/nand/sh_flctl.c +++ b/drivers/mtd/nand/sh_flctl.c @@ -688,6 +688,8 @@ static void flctl_select_chip(struct mtd_info *mtd, int chipnr) break; case 0: writel(flctl->flcmncr_val | CE0_ENABLE, FLCMNCR(flctl)); + if (flctl->holden) + writel(HOLDEN, FLHOLDCR(flctl)); break; default: BUG(); @@ -845,6 +847,7 @@ static int __devinit flctl_probe(struct platform_device *pdev) flctl->pdev = pdev; flctl->flcmncr_val = pdata->flcmncr_val; flctl->hwecc = pdata->has_hwecc; + flctl->holden = pdata->use_holden; nand->options = NAND_NO_AUTOINCR; diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h index 107fd8a..6046443 100644 --- a/include/linux/mtd/sh_flctl.h +++ b/include/linux/mtd/sh_flctl.h @@ -38,6 +38,7 @@ #define FLDTFIFO(f) (f->reg + 0x24) #define FLECFIFO(f) (f->reg + 0x28) #define FLTRCR(f) (f->reg + 0x2C) +#define FLHOLDCR(f) (f->reg + 0x38) #define FL4ECCRESULT0(f) (f->reg + 0x80) #define FL4ECCRESULT1(f) (f->reg + 0x84) #define FL4ECCRESULT2(f) (f->reg + 0x88) @@ -109,6 +110,15 @@ #define TRSTRT (0x1 << 0) /* translation start */ #define TREND (0x1 << 1) /* translation end */ +/* + * FLHOLDCR control bits + * + * HOLDEN: Bus Occupancy Enable (inverted) + * Enable this bit when the external bus might be used in between transfers. + * If not set and the bus gets used by other modules, a deadlock occurs. + */ +#define HOLDEN (0x1 << 0) + /* FL4ECCCR control bits */ #define _4ECCFA (0x1 << 2) /* 4 symbols correct fault */ #define _4ECCEND (0x1 << 1) /* 4 symbols end */ @@ -138,6 +148,7 @@ struct sh_flctl { unsigned page_size:1; /* NAND page size (0 = 512, 1 = 2048) */ unsigned hwecc:1; /* Hardware ECC (0 = disabled, 1 = enabled) */ + unsigned holden:1; /* Hardware has FLHOLDCR and HOLDEN is set */ }; struct sh_flctl_platform_data { @@ -146,6 +157,7 @@ struct sh_flctl_platform_data { unsigned long flcmncr_val; unsigned has_hwecc:1; + unsigned use_holden:1; }; static inline struct sh_flctl *mtd_to_flctl(struct mtd_info *mtdinfo)
Add a register used in new FLCTL hardware and a feature flag for it. Signed-off-by: Bastian Hecht <hechtb@gmail.com> --- changelog: the write to the register has been moved due to patch 5. drivers/mtd/nand/sh_flctl.c | 3 +++ include/linux/mtd/sh_flctl.h | 12 ++++++++++++ 2 files changed, 15 insertions(+), 0 deletions(-)