| Message ID | 1472739890-3384-3-git-send-email-alex.lemberg@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 2016/9/1 22:24, alex lemberg wrote: > Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> It would be nice to add some change log.:) This part is still on my TODO list, so very nice to see you provide patches for this. But I don't see we reach an agreement at that time for whether it would be sane to set this bit for kernel. I remember Adrian's opinion was to let userspace take over the ownership of this, and Ulf maybe had no hard opinon for this(Ulf, right? :)). > --- > drivers/mmc/core/mmc.c | 6 ++++++ > include/linux/mmc/card.h | 1 + > include/linux/mmc/mmc.h | 1 + > 3 files changed, 8 insertions(+) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index f2d185c..e2e987f 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -531,6 +531,12 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) > if (!card->ext_csd.man_bkops_en) > pr_debug("%s: MAN_BKOPS_EN bit is not set\n", > mmc_hostname(card->host)); > + card->ext_csd.auto_bkops_en = > + (ext_csd[EXT_CSD_BKOPS_EN] & > + EXT_CSD_AUTO_BKOPS_MASK); > + if (!card->ext_csd.auto_bkops_en) > + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", > + mmc_hostname(card->host)); > } > > /* check whether the eMMC card supports HPI */ > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 73fad83..aaedb68 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -84,6 +84,7 @@ struct mmc_ext_csd { > unsigned int hpi_cmd; /* cmd used as HPI */ > bool bkops; /* background support bit */ > bool man_bkops_en; /* manual bkops enable bit */ > + bool auto_bkops_en; /* auto bkops enable bit */ > unsigned int data_sector_size; /* 512 bytes or 4KB */ > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ > unsigned int boot_ro_lock; /* ro lock support */ > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h > index c376209..0fe3908 100644 > --- a/include/linux/mmc/mmc.h > +++ b/include/linux/mmc/mmc.h > @@ -436,6 +436,7 @@ struct _mmc_csd { > * BKOPS modes > */ > #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 > +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 > > /* > * MMC_SWITCH access modes >
Hi Shawn, Thanks for your comments. On 9/2/16, 10:50 AM, "Shawn Lin" <shawn.lin@rock-chips.com> wrote: >Hi, > >On 2016/9/1 22:24, alex lemberg wrote: >> Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> > >It would be nice to add some change log.:) Will be add in case we release additional patch version. > >This part is still on my TODO list, so very nice to see >you provide patches for this. But I don't see we reach >an agreement at that time for whether it would be sane to >set this bit for kernel. I remember Adrian's opinion was to >let userspace take over the ownership of this, and Ulf maybe >had no hard opinon for this(Ulf, right? :)). I think you are referring to the “[pre]set AUTO BKOPS” functionality to the device. Am I right? In this patch the value of AUTO_BKOPS_EN bit is read from the device EXT_CSD and set to the card->ext_csd structure. > > >> --- >> drivers/mmc/core/mmc.c | 6 ++++++ >> include/linux/mmc/card.h | 1 + >> include/linux/mmc/mmc.h | 1 + >> 3 files changed, 8 insertions(+) >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index f2d185c..e2e987f 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -531,6 +531,12 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) >> if (!card->ext_csd.man_bkops_en) >> pr_debug("%s: MAN_BKOPS_EN bit is not set\n", >> mmc_hostname(card->host)); >> + card->ext_csd.auto_bkops_en = >> + (ext_csd[EXT_CSD_BKOPS_EN] & >> + EXT_CSD_AUTO_BKOPS_MASK); >> + if (!card->ext_csd.auto_bkops_en) >> + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", >> + mmc_hostname(card->host)); >> } >> >> /* check whether the eMMC card supports HPI */ >> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >> index 73fad83..aaedb68 100644 >> --- a/include/linux/mmc/card.h >> +++ b/include/linux/mmc/card.h >> @@ -84,6 +84,7 @@ struct mmc_ext_csd { >> unsigned int hpi_cmd; /* cmd used as HPI */ >> bool bkops; /* background support bit */ >> bool man_bkops_en; /* manual bkops enable bit */ >> + bool auto_bkops_en; /* auto bkops enable bit */ >> unsigned int data_sector_size; /* 512 bytes or 4KB */ >> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ >> unsigned int boot_ro_lock; /* ro lock support */ >> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >> index c376209..0fe3908 100644 >> --- a/include/linux/mmc/mmc.h >> +++ b/include/linux/mmc/mmc.h >> @@ -436,6 +436,7 @@ struct _mmc_csd { >> * BKOPS modes >> */ >> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 >> +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 >> >> /* >> * MMC_SWITCH access modes >> > > >-- >Best Regards >Shawn Lin >
On 1 September 2016 at 16:24, alex lemberg <alex.lemberg@sandisk.com> wrote: > Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> > --- > drivers/mmc/core/mmc.c | 6 ++++++ > include/linux/mmc/card.h | 1 + > include/linux/mmc/mmc.h | 1 + > 3 files changed, 8 insertions(+) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index f2d185c..e2e987f 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -531,6 +531,12 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) > if (!card->ext_csd.man_bkops_en) > pr_debug("%s: MAN_BKOPS_EN bit is not set\n", > mmc_hostname(card->host)); > + card->ext_csd.auto_bkops_en = > + (ext_csd[EXT_CSD_BKOPS_EN] & > + EXT_CSD_AUTO_BKOPS_MASK); > + if (!card->ext_csd.auto_bkops_en) > + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", > + mmc_hostname(card->host)); According to discussions here [1], I am wondering whether we actually should inverse the logic for when printing the debug message. Isn't it more relevant to know when the bits *are* set than rather when they aren't? > } > > /* check whether the eMMC card supports HPI */ > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 73fad83..aaedb68 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -84,6 +84,7 @@ struct mmc_ext_csd { > unsigned int hpi_cmd; /* cmd used as HPI */ > bool bkops; /* background support bit */ > bool man_bkops_en; /* manual bkops enable bit */ > + bool auto_bkops_en; /* auto bkops enable bit */ > unsigned int data_sector_size; /* 512 bytes or 4KB */ > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ > unsigned int boot_ro_lock; /* ro lock support */ > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h > index c376209..0fe3908 100644 > --- a/include/linux/mmc/mmc.h > +++ b/include/linux/mmc/mmc.h > @@ -436,6 +436,7 @@ struct _mmc_csd { > * BKOPS modes > */ > #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 > +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 > > /* > * MMC_SWITCH access modes > -- > 1.9.1 > Kind regards Uffe [1] http://www.spinics.net/lists/linux-mmc/msg39553.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On 10/13/16, 10:36 AM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote: >On 1 September 2016 at 16:24, alex lemberg <alex.lemberg@sandisk.com> wrote: >> Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> >> --- >> drivers/mmc/core/mmc.c | 6 ++++++ >> include/linux/mmc/card.h | 1 + >> include/linux/mmc/mmc.h | 1 + >> 3 files changed, 8 insertions(+) >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index f2d185c..e2e987f 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -531,6 +531,12 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) >> if (!card->ext_csd.man_bkops_en) >> pr_debug("%s: MAN_BKOPS_EN bit is not set\n", >> mmc_hostname(card->host)); >> + card->ext_csd.auto_bkops_en = >> + (ext_csd[EXT_CSD_BKOPS_EN] & >> + EXT_CSD_AUTO_BKOPS_MASK); >> + if (!card->ext_csd.auto_bkops_en) >> + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", >> + mmc_hostname(card->host)); > >According to discussions here [1], I am wondering whether we actually >should inverse the logic for when printing the debug message. > >Isn't it more relevant to know when the bits *are* set than rather >when they aren't? Agree, since we have two types of BKOPS now, it more makes sense to print a currently enabled/set type or BKOPS bits. I will change and re-submit. > >> } >> >> /* check whether the eMMC card supports HPI */ >> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >> index 73fad83..aaedb68 100644 >> --- a/include/linux/mmc/card.h >> +++ b/include/linux/mmc/card.h >> @@ -84,6 +84,7 @@ struct mmc_ext_csd { >> unsigned int hpi_cmd; /* cmd used as HPI */ >> bool bkops; /* background support bit */ >> bool man_bkops_en; /* manual bkops enable bit */ >> + bool auto_bkops_en; /* auto bkops enable bit */ >> unsigned int data_sector_size; /* 512 bytes or 4KB */ >> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ >> unsigned int boot_ro_lock; /* ro lock support */ >> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >> index c376209..0fe3908 100644 >> --- a/include/linux/mmc/mmc.h >> +++ b/include/linux/mmc/mmc.h >> @@ -436,6 +436,7 @@ struct _mmc_csd { >> * BKOPS modes >> */ >> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 >> +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 >> >> /* >> * MMC_SWITCH access modes >> -- >> 1.9.1 >> > >Kind regards >Uffe > >[1] >http://www.spinics.net/lists/linux-mmc/msg39553.html
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index f2d185c..e2e987f 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -531,6 +531,12 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) if (!card->ext_csd.man_bkops_en) pr_debug("%s: MAN_BKOPS_EN bit is not set\n", mmc_hostname(card->host)); + card->ext_csd.auto_bkops_en = + (ext_csd[EXT_CSD_BKOPS_EN] & + EXT_CSD_AUTO_BKOPS_MASK); + if (!card->ext_csd.auto_bkops_en) + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", + mmc_hostname(card->host)); } /* check whether the eMMC card supports HPI */ diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 73fad83..aaedb68 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -84,6 +84,7 @@ struct mmc_ext_csd { unsigned int hpi_cmd; /* cmd used as HPI */ bool bkops; /* background support bit */ bool man_bkops_en; /* manual bkops enable bit */ + bool auto_bkops_en; /* auto bkops enable bit */ unsigned int data_sector_size; /* 512 bytes or 4KB */ unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ unsigned int boot_ro_lock; /* ro lock support */ diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h index c376209..0fe3908 100644 --- a/include/linux/mmc/mmc.h +++ b/include/linux/mmc/mmc.h @@ -436,6 +436,7 @@ struct _mmc_csd { * BKOPS modes */ #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 /* * MMC_SWITCH access modes
Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> --- drivers/mmc/core/mmc.c | 6 ++++++ include/linux/mmc/card.h | 1 + include/linux/mmc/mmc.h | 1 + 3 files changed, 8 insertions(+)