* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
@ 2021-02-15 3:09 kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-02-15 3:09 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9527 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210215003249.GA12303@lupo-laptop>
References: <20210215003249.GA12303@lupo-laptop>
TO: Luca Porzio <porzio@gmail.com>
Hi Luca,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on next-20210212]
[cannot apply to linus/master mmc/mmc-next v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luca-Porzio/Support-temporarily-disable-of-the-CMDQ-mode/20210215-083730
base: 07f7e57c63aaa2afb4ea31edef05e08699a63a00
:::::: branch date: 3 hours ago
:::::: commit date: 3 hours ago
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
"cppcheck warnings: (new ones prefixed by >>)"
>> drivers/mmc/core/mmc.c:870:9: warning: Uninitialized variable: err [uninitvar]
return err;
^
cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
>> drivers/mmc/core/mmc.c:885:26: warning: Either the condition '!card' is redundant or there is possible null pointer dereference: card. [nullPointerRedundantCheck]
struct mmc_host *host = card->host;
^
drivers/mmc/core/mmc.c:889:6: note: Assuming that condition '!card' is not redundant
if (!card || kstrtoul(buf, 0, &enable))
^
drivers/mmc/core/mmc.c:885:26: note: Null pointer dereference
struct mmc_host *host = card->host;
^
vim +885 drivers/mmc/core/mmc.c
07a9ce0e702520 Luca Porzio 2021-02-15 797
07a9ce0e702520 Luca Porzio 2021-02-15 798
07a9ce0e702520 Luca Porzio 2021-02-15 799 /* Setup command queue mode and CQE if underling hw supports it
07a9ce0e702520 Luca Porzio 2021-02-15 800 * and assuming force_disable_cmdq has not been set.
07a9ce0e702520 Luca Porzio 2021-02-15 801 */
07a9ce0e702520 Luca Porzio 2021-02-15 802 static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
07a9ce0e702520 Luca Porzio 2021-02-15 803 {
07a9ce0e702520 Luca Porzio 2021-02-15 804 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 805
07a9ce0e702520 Luca Porzio 2021-02-15 806 /* Check HW support */
07a9ce0e702520 Luca Porzio 2021-02-15 807 if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
07a9ce0e702520 Luca Porzio 2021-02-15 808 card->force_disable_cmdq = true;
07a9ce0e702520 Luca Porzio 2021-02-15 809
07a9ce0e702520 Luca Porzio 2021-02-15 810 /* Enable/Disable CMDQ mode */
07a9ce0e702520 Luca Porzio 2021-02-15 811 if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 812 err = mmc_cmdq_enable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 813 if (err && err != -EBADMSG)
07a9ce0e702520 Luca Porzio 2021-02-15 814 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 815 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 816 pr_warn("%s: Enabling CMDQ failed\n",
07a9ce0e702520 Luca Porzio 2021-02-15 817 mmc_hostname(card->host));
07a9ce0e702520 Luca Porzio 2021-02-15 818 card->ext_csd.cmdq_support = false;
07a9ce0e702520 Luca Porzio 2021-02-15 819 card->ext_csd.cmdq_depth = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 820 }
07a9ce0e702520 Luca Porzio 2021-02-15 821
07a9ce0e702520 Luca Porzio 2021-02-15 822 } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 823 err = mmc_cmdq_disable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 824 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 825 pr_warn("%s: Disabling CMDQ failed, error %d\n",
07a9ce0e702520 Luca Porzio 2021-02-15 826 mmc_hostname(card->host), err);
07a9ce0e702520 Luca Porzio 2021-02-15 827 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 828 }
07a9ce0e702520 Luca Porzio 2021-02-15 829 }
07a9ce0e702520 Luca Porzio 2021-02-15 830
07a9ce0e702520 Luca Porzio 2021-02-15 831 /*
07a9ce0e702520 Luca Porzio 2021-02-15 832 * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
07a9ce0e702520 Luca Porzio 2021-02-15 833 * disabled for a time, so a flag is needed to indicate to re-enable the
07a9ce0e702520 Luca Porzio 2021-02-15 834 * Command Queue.
07a9ce0e702520 Luca Porzio 2021-02-15 835 */
07a9ce0e702520 Luca Porzio 2021-02-15 836 card->reenable_cmdq = card->ext_csd.cmdq_en;
07a9ce0e702520 Luca Porzio 2021-02-15 837
07a9ce0e702520 Luca Porzio 2021-02-15 838 /* Enable/Disable Host CQE */
07a9ce0e702520 Luca Porzio 2021-02-15 839 if (!card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 840
07a9ce0e702520 Luca Porzio 2021-02-15 841 if (host->cqe_ops && !host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 842 err = host->cqe_ops->cqe_enable(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 843 if (!err) {
07a9ce0e702520 Luca Porzio 2021-02-15 844 host->cqe_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 845
07a9ce0e702520 Luca Porzio 2021-02-15 846 if (card->ext_csd.cmdq_en) {
07a9ce0e702520 Luca Porzio 2021-02-15 847 pr_info("%s: Command Queue Engine enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 848 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 849 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 850 host->hsq_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 851 pr_info("%s: Host Software Queue enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 852 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 853 }
07a9ce0e702520 Luca Porzio 2021-02-15 854 }
07a9ce0e702520 Luca Porzio 2021-02-15 855 }
07a9ce0e702520 Luca Porzio 2021-02-15 856
07a9ce0e702520 Luca Porzio 2021-02-15 857 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 858
07a9ce0e702520 Luca Porzio 2021-02-15 859 if (host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 860 host->cqe_ops->cqe_disable(host);
07a9ce0e702520 Luca Porzio 2021-02-15 861 host->cqe_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 862 pr_info("%s: Command Queue Engine disabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 863 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 864 }
07a9ce0e702520 Luca Porzio 2021-02-15 865
07a9ce0e702520 Luca Porzio 2021-02-15 866 host->hsq_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 867 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 868 }
07a9ce0e702520 Luca Porzio 2021-02-15 869
07a9ce0e702520 Luca Porzio 2021-02-15 @870 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 871 }
07a9ce0e702520 Luca Porzio 2021-02-15 872
07a9ce0e702520 Luca Porzio 2021-02-15 873
07a9ce0e702520 Luca Porzio 2021-02-15 874 static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
07a9ce0e702520 Luca Porzio 2021-02-15 875 {
07a9ce0e702520 Luca Porzio 2021-02-15 876 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 877
07a9ce0e702520 Luca Porzio 2021-02-15 878 return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
07a9ce0e702520 Luca Porzio 2021-02-15 879 }
07a9ce0e702520 Luca Porzio 2021-02-15 880
07a9ce0e702520 Luca Porzio 2021-02-15 881 static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
07a9ce0e702520 Luca Porzio 2021-02-15 882 const char *buf, size_t count)
07a9ce0e702520 Luca Porzio 2021-02-15 883 {
07a9ce0e702520 Luca Porzio 2021-02-15 884 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 @885 struct mmc_host *host = card->host;
07a9ce0e702520 Luca Porzio 2021-02-15 886 unsigned long enable;
07a9ce0e702520 Luca Porzio 2021-02-15 887 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 888
07a9ce0e702520 Luca Porzio 2021-02-15 889 if (!card || kstrtoul(buf, 0, &enable))
07a9ce0e702520 Luca Porzio 2021-02-15 890 return -EINVAL;
07a9ce0e702520 Luca Porzio 2021-02-15 891 if (!card->ext_csd.cmdq_support)
07a9ce0e702520 Luca Porzio 2021-02-15 892 return -EOPNOTSUPP;
07a9ce0e702520 Luca Porzio 2021-02-15 893
07a9ce0e702520 Luca Porzio 2021-02-15 894 enable = !!enable;
07a9ce0e702520 Luca Porzio 2021-02-15 895 if (enable == card->ext_csd.cmdq_en)
07a9ce0e702520 Luca Porzio 2021-02-15 896 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 897
07a9ce0e702520 Luca Porzio 2021-02-15 898 mmc_get_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 899 card->force_disable_cmdq = !enable;
07a9ce0e702520 Luca Porzio 2021-02-15 900 err = mmc_cmdq_setup(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 901 mmc_put_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 902
07a9ce0e702520 Luca Porzio 2021-02-15 903 if (err)
07a9ce0e702520 Luca Porzio 2021-02-15 904 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 905 else
07a9ce0e702520 Luca Porzio 2021-02-15 906 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 907 }
07a9ce0e702520 Luca Porzio 2021-02-15 908
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
@ 2021-02-15 2:21 kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-02-15 2:21 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9634 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210215003249.GA12303@lupo-laptop>
References: <20210215003249.GA12303@lupo-laptop>
TO: Luca Porzio <porzio@gmail.com>
Hi Luca,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on next-20210212]
[cannot apply to linus/master mmc/mmc-next v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luca-Porzio/Support-temporarily-disable-of-the-CMDQ-mode/20210215-083730
base: 07f7e57c63aaa2afb4ea31edef05e08699a63a00
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago
config: i386-randconfig-m021-20210215 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/mmc/core/mmc.c:870 mmc_cmdq_setup() error: uninitialized symbol 'err'.
drivers/mmc/core/mmc.c:889 cmdq_en_store() warn: variable dereferenced before check 'card' (see line 885)
Old smatch warnings:
drivers/mmc/core/mmc.c:774 mmc_erase_size_show() warn: should 'card->erase_size << 9' be a 64 bit type?
drivers/mmc/core/mmc.c:775 mmc_preferred_erase_size_show() warn: should 'card->pref_erase << 9' be a 64 bit type?
drivers/mmc/core/mmc.c:1072 mmc_select_powerclass() warn: should '1 << bus_width' be a 64 bit type?
drivers/mmc/core/mmc.c:1159 mmc_select_bus_width() warn: should '1 << bus_width' be a 64 bit type?
drivers/mmc/core/mmc.c:1211 mmc_select_hs_ddr() warn: should '1 << bus_width' be a 64 bit type?
vim +/err +870 drivers/mmc/core/mmc.c
07a9ce0e702520 Luca Porzio 2021-02-15 797
07a9ce0e702520 Luca Porzio 2021-02-15 798
07a9ce0e702520 Luca Porzio 2021-02-15 799 /* Setup command queue mode and CQE if underling hw supports it
07a9ce0e702520 Luca Porzio 2021-02-15 800 * and assuming force_disable_cmdq has not been set.
07a9ce0e702520 Luca Porzio 2021-02-15 801 */
07a9ce0e702520 Luca Porzio 2021-02-15 802 static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
07a9ce0e702520 Luca Porzio 2021-02-15 803 {
07a9ce0e702520 Luca Porzio 2021-02-15 804 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 805
07a9ce0e702520 Luca Porzio 2021-02-15 806 /* Check HW support */
07a9ce0e702520 Luca Porzio 2021-02-15 807 if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
07a9ce0e702520 Luca Porzio 2021-02-15 808 card->force_disable_cmdq = true;
07a9ce0e702520 Luca Porzio 2021-02-15 809
07a9ce0e702520 Luca Porzio 2021-02-15 810 /* Enable/Disable CMDQ mode */
07a9ce0e702520 Luca Porzio 2021-02-15 811 if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 812 err = mmc_cmdq_enable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 813 if (err && err != -EBADMSG)
07a9ce0e702520 Luca Porzio 2021-02-15 814 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 815 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 816 pr_warn("%s: Enabling CMDQ failed\n",
07a9ce0e702520 Luca Porzio 2021-02-15 817 mmc_hostname(card->host));
07a9ce0e702520 Luca Porzio 2021-02-15 818 card->ext_csd.cmdq_support = false;
07a9ce0e702520 Luca Porzio 2021-02-15 819 card->ext_csd.cmdq_depth = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 820 }
07a9ce0e702520 Luca Porzio 2021-02-15 821
07a9ce0e702520 Luca Porzio 2021-02-15 822 } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 823 err = mmc_cmdq_disable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 824 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 825 pr_warn("%s: Disabling CMDQ failed, error %d\n",
07a9ce0e702520 Luca Porzio 2021-02-15 826 mmc_hostname(card->host), err);
07a9ce0e702520 Luca Porzio 2021-02-15 827 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 828 }
07a9ce0e702520 Luca Porzio 2021-02-15 829 }
07a9ce0e702520 Luca Porzio 2021-02-15 830
07a9ce0e702520 Luca Porzio 2021-02-15 831 /*
07a9ce0e702520 Luca Porzio 2021-02-15 832 * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
07a9ce0e702520 Luca Porzio 2021-02-15 833 * disabled for a time, so a flag is needed to indicate to re-enable the
07a9ce0e702520 Luca Porzio 2021-02-15 834 * Command Queue.
07a9ce0e702520 Luca Porzio 2021-02-15 835 */
07a9ce0e702520 Luca Porzio 2021-02-15 836 card->reenable_cmdq = card->ext_csd.cmdq_en;
07a9ce0e702520 Luca Porzio 2021-02-15 837
07a9ce0e702520 Luca Porzio 2021-02-15 838 /* Enable/Disable Host CQE */
07a9ce0e702520 Luca Porzio 2021-02-15 839 if (!card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 840
07a9ce0e702520 Luca Porzio 2021-02-15 841 if (host->cqe_ops && !host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 842 err = host->cqe_ops->cqe_enable(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 843 if (!err) {
07a9ce0e702520 Luca Porzio 2021-02-15 844 host->cqe_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 845
07a9ce0e702520 Luca Porzio 2021-02-15 846 if (card->ext_csd.cmdq_en) {
07a9ce0e702520 Luca Porzio 2021-02-15 847 pr_info("%s: Command Queue Engine enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 848 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 849 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 850 host->hsq_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 851 pr_info("%s: Host Software Queue enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 852 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 853 }
07a9ce0e702520 Luca Porzio 2021-02-15 854 }
07a9ce0e702520 Luca Porzio 2021-02-15 855 }
07a9ce0e702520 Luca Porzio 2021-02-15 856
07a9ce0e702520 Luca Porzio 2021-02-15 857 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 858
07a9ce0e702520 Luca Porzio 2021-02-15 859 if (host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 860 host->cqe_ops->cqe_disable(host);
07a9ce0e702520 Luca Porzio 2021-02-15 861 host->cqe_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 862 pr_info("%s: Command Queue Engine disabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 863 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 864 }
07a9ce0e702520 Luca Porzio 2021-02-15 865
07a9ce0e702520 Luca Porzio 2021-02-15 866 host->hsq_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 867 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 868 }
07a9ce0e702520 Luca Porzio 2021-02-15 869
07a9ce0e702520 Luca Porzio 2021-02-15 @870 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 871 }
07a9ce0e702520 Luca Porzio 2021-02-15 872
07a9ce0e702520 Luca Porzio 2021-02-15 873
07a9ce0e702520 Luca Porzio 2021-02-15 874 static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
07a9ce0e702520 Luca Porzio 2021-02-15 875 {
07a9ce0e702520 Luca Porzio 2021-02-15 876 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 877
07a9ce0e702520 Luca Porzio 2021-02-15 878 return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
07a9ce0e702520 Luca Porzio 2021-02-15 879 }
07a9ce0e702520 Luca Porzio 2021-02-15 880
07a9ce0e702520 Luca Porzio 2021-02-15 881 static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
07a9ce0e702520 Luca Porzio 2021-02-15 882 const char *buf, size_t count)
07a9ce0e702520 Luca Porzio 2021-02-15 883 {
07a9ce0e702520 Luca Porzio 2021-02-15 884 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 @885 struct mmc_host *host = card->host;
07a9ce0e702520 Luca Porzio 2021-02-15 886 unsigned long enable;
07a9ce0e702520 Luca Porzio 2021-02-15 887 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 888
07a9ce0e702520 Luca Porzio 2021-02-15 @889 if (!card || kstrtoul(buf, 0, &enable))
07a9ce0e702520 Luca Porzio 2021-02-15 890 return -EINVAL;
07a9ce0e702520 Luca Porzio 2021-02-15 891 if (!card->ext_csd.cmdq_support)
07a9ce0e702520 Luca Porzio 2021-02-15 892 return -EOPNOTSUPP;
07a9ce0e702520 Luca Porzio 2021-02-15 893
07a9ce0e702520 Luca Porzio 2021-02-15 894 enable = !!enable;
07a9ce0e702520 Luca Porzio 2021-02-15 895 if (enable == card->ext_csd.cmdq_en)
07a9ce0e702520 Luca Porzio 2021-02-15 896 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 897
07a9ce0e702520 Luca Porzio 2021-02-15 898 mmc_get_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 899 card->force_disable_cmdq = !enable;
07a9ce0e702520 Luca Porzio 2021-02-15 900 err = mmc_cmdq_setup(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 901 mmc_put_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 902
07a9ce0e702520 Luca Porzio 2021-02-15 903 if (err)
07a9ce0e702520 Luca Porzio 2021-02-15 904 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 905 else
07a9ce0e702520 Luca Porzio 2021-02-15 906 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 907 }
07a9ce0e702520 Luca Porzio 2021-02-15 908
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38219 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* [RFC PATCH 2/2] Make cmdq_en attribute writeable
@ 2021-02-15 0:32 Luca Porzio
2021-02-15 13:49 ` Dan Carpenter
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Luca Porzio @ 2021-02-15 0:32 UTC (permalink / raw)
To: linux-mmc; +Cc: Zhan Liu, Luca Porzio
cmdq_en attribute in sysfs now can now be written.
When 0 is written:
CMDQ is disabled and kept disabled across device reboots.
When 1 is written:
CMDQ mode is instantly reneabled (if supported).
Signed-off-by: Luca Porzio <lporzio@micron.com>
Signed-off-by: Zhan Liu <zliua@micron.com>
---
drivers/mmc/core/mmc.c | 152 ++++++++++++++++++++++++++++++---------
include/linux/mmc/card.h | 1 +
2 files changed, 118 insertions(+), 35 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 0d80b72ddde8..5c7d5bac5c00 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -794,7 +794,120 @@ MMC_DEV_ATTR(enhanced_rpmb_supported, "%#x\n",
MMC_DEV_ATTR(rel_sectors, "%#x\n", card->ext_csd.rel_sectors);
MMC_DEV_ATTR(ocr, "0x%08x\n", card->ocr);
MMC_DEV_ATTR(rca, "0x%04x\n", card->rca);
-MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en);
+
+
+/* Setup command queue mode and CQE if underling hw supports it
+ * and assuming force_disable_cmdq has not been set.
+ */
+static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
+{
+ int err;
+
+ /* Check HW support */
+ if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
+ card->force_disable_cmdq = true;
+
+ /* Enable/Disable CMDQ mode */
+ if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
+ err = mmc_cmdq_enable(card);
+ if (err && err != -EBADMSG)
+ return err;
+ if (err) {
+ pr_warn("%s: Enabling CMDQ failed\n",
+ mmc_hostname(card->host));
+ card->ext_csd.cmdq_support = false;
+ card->ext_csd.cmdq_depth = 0;
+ }
+
+ } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
+ err = mmc_cmdq_disable(card);
+ if (err) {
+ pr_warn("%s: Disabling CMDQ failed, error %d\n",
+ mmc_hostname(card->host), err);
+ err = 0;
+ }
+ }
+
+ /*
+ * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
+ * disabled for a time, so a flag is needed to indicate to re-enable the
+ * Command Queue.
+ */
+ card->reenable_cmdq = card->ext_csd.cmdq_en;
+
+ /* Enable/Disable Host CQE */
+ if (!card->force_disable_cmdq) {
+
+ if (host->cqe_ops && !host->cqe_enabled) {
+ err = host->cqe_ops->cqe_enable(host, card);
+ if (!err) {
+ host->cqe_enabled = true;
+
+ if (card->ext_csd.cmdq_en) {
+ pr_info("%s: Command Queue Engine enabled\n",
+ mmc_hostname(host));
+ } else {
+ host->hsq_enabled = true;
+ pr_info("%s: Host Software Queue enabled\n",
+ mmc_hostname(host));
+ }
+ }
+ }
+
+ } else {
+
+ if (host->cqe_enabled) {
+ host->cqe_ops->cqe_disable(host);
+ host->cqe_enabled = false;
+ pr_info("%s: Command Queue Engine disabled\n",
+ mmc_hostname(host));
+ }
+
+ host->hsq_enabled = false;
+ err = 0;
+ }
+
+ return err;
+}
+
+
+static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct mmc_card *card = mmc_dev_to_card(dev);
+
+ return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
+}
+
+static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct mmc_card *card = mmc_dev_to_card(dev);
+ struct mmc_host *host = card->host;
+ unsigned long enable;
+ int err;
+
+ if (!card || kstrtoul(buf, 0, &enable))
+ return -EINVAL;
+ if (!card->ext_csd.cmdq_support)
+ return -EOPNOTSUPP;
+
+ enable = !!enable;
+ if (enable == card->ext_csd.cmdq_en)
+ return count;
+
+ mmc_get_card(card, NULL);
+ card->force_disable_cmdq = !enable;
+ err = mmc_cmdq_setup(host, card);
+ mmc_put_card(card, NULL);
+
+ if (err)
+ return err;
+ else
+ return count;
+}
+
+static DEVICE_ATTR_RW(cmdq_en);
+
static ssize_t mmc_fwrev_show(struct device *dev,
struct device_attribute *attr,
@@ -1838,40 +1951,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
* Enable Command Queue if supported. Note that Packed Commands cannot
* be used with Command Queue.
*/
- card->ext_csd.cmdq_en = false;
- if (card->ext_csd.cmdq_support && host->caps2 & MMC_CAP2_CQE) {
- err = mmc_cmdq_enable(card);
- if (err && err != -EBADMSG)
- goto free_card;
- if (err) {
- pr_warn("%s: Enabling CMDQ failed\n",
- mmc_hostname(card->host));
- card->ext_csd.cmdq_support = false;
- card->ext_csd.cmdq_depth = 0;
- }
- }
- /*
- * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
- * disabled for a time, so a flag is needed to indicate to re-enable the
- * Command Queue.
- */
- card->reenable_cmdq = card->ext_csd.cmdq_en;
-
- if (host->cqe_ops && !host->cqe_enabled) {
- err = host->cqe_ops->cqe_enable(host, card);
- if (!err) {
- host->cqe_enabled = true;
-
- if (card->ext_csd.cmdq_en) {
- pr_info("%s: Command Queue Engine enabled\n",
- mmc_hostname(host));
- } else {
- host->hsq_enabled = true;
- pr_info("%s: Host Software Queue enabled\n",
- mmc_hostname(host));
- }
- }
- }
+ err = mmc_cmdq_setup(host, card);
+ if (err)
+ goto free_card;
if (host->caps2 & MMC_CAP2_AVOID_3_3V &&
host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index f9ad35dd6012..e554bb0cf722 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -272,6 +272,7 @@ struct mmc_card {
#define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */
bool reenable_cmdq; /* Re-enable Command Queue */
+ bool force_disable_cmdq; /* Keep Command Queue disabled */
unsigned int erase_size; /* erase size in sectors */
unsigned int erase_shift; /* if erase unit is power 2 */
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
2021-02-15 0:32 Luca Porzio
@ 2021-02-15 13:49 ` Dan Carpenter
2021-02-17 13:04 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2021-02-15 13:49 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 8357 bytes --]
Hi Luca,
url: https://github.com/0day-ci/linux/commits/Luca-Porzio/Support-temporarily-disable-of-the-CMDQ-mode/20210215-083730
base: 07f7e57c63aaa2afb4ea31edef05e08699a63a00
config: i386-randconfig-m021-20210215 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/mmc/core/mmc.c:870 mmc_cmdq_setup() error: uninitialized symbol 'err'.
drivers/mmc/core/mmc.c:889 cmdq_en_store() warn: variable dereferenced before check 'card' (see line 885)
vim +/err +870 drivers/mmc/core/mmc.c
07a9ce0e702520 Luca Porzio 2021-02-15 802 static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
07a9ce0e702520 Luca Porzio 2021-02-15 803 {
07a9ce0e702520 Luca Porzio 2021-02-15 804 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 805
07a9ce0e702520 Luca Porzio 2021-02-15 806 /* Check HW support */
07a9ce0e702520 Luca Porzio 2021-02-15 807 if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
07a9ce0e702520 Luca Porzio 2021-02-15 808 card->force_disable_cmdq = true;
07a9ce0e702520 Luca Porzio 2021-02-15 809
07a9ce0e702520 Luca Porzio 2021-02-15 810 /* Enable/Disable CMDQ mode */
07a9ce0e702520 Luca Porzio 2021-02-15 811 if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 812 err = mmc_cmdq_enable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 813 if (err && err != -EBADMSG)
07a9ce0e702520 Luca Porzio 2021-02-15 814 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 815 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 816 pr_warn("%s: Enabling CMDQ failed\n",
07a9ce0e702520 Luca Porzio 2021-02-15 817 mmc_hostname(card->host));
07a9ce0e702520 Luca Porzio 2021-02-15 818 card->ext_csd.cmdq_support = false;
07a9ce0e702520 Luca Porzio 2021-02-15 819 card->ext_csd.cmdq_depth = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 820 }
07a9ce0e702520 Luca Porzio 2021-02-15 821
07a9ce0e702520 Luca Porzio 2021-02-15 822 } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 823 err = mmc_cmdq_disable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 824 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 825 pr_warn("%s: Disabling CMDQ failed, error %d\n",
07a9ce0e702520 Luca Porzio 2021-02-15 826 mmc_hostname(card->host), err);
07a9ce0e702520 Luca Porzio 2021-02-15 827 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 828 }
07a9ce0e702520 Luca Porzio 2021-02-15 829 }
"err not set on else path"
07a9ce0e702520 Luca Porzio 2021-02-15 830
07a9ce0e702520 Luca Porzio 2021-02-15 831 /*
07a9ce0e702520 Luca Porzio 2021-02-15 832 * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
07a9ce0e702520 Luca Porzio 2021-02-15 833 * disabled for a time, so a flag is needed to indicate to re-enable the
07a9ce0e702520 Luca Porzio 2021-02-15 834 * Command Queue.
07a9ce0e702520 Luca Porzio 2021-02-15 835 */
07a9ce0e702520 Luca Porzio 2021-02-15 836 card->reenable_cmdq = card->ext_csd.cmdq_en;
07a9ce0e702520 Luca Porzio 2021-02-15 837
07a9ce0e702520 Luca Porzio 2021-02-15 838 /* Enable/Disable Host CQE */
07a9ce0e702520 Luca Porzio 2021-02-15 839 if (!card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 840
07a9ce0e702520 Luca Porzio 2021-02-15 841 if (host->cqe_ops && !host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 842 err = host->cqe_ops->cqe_enable(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 843 if (!err) {
07a9ce0e702520 Luca Porzio 2021-02-15 844 host->cqe_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 845
07a9ce0e702520 Luca Porzio 2021-02-15 846 if (card->ext_csd.cmdq_en) {
07a9ce0e702520 Luca Porzio 2021-02-15 847 pr_info("%s: Command Queue Engine enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 848 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 849 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 850 host->hsq_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 851 pr_info("%s: Host Software Queue enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 852 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 853 }
07a9ce0e702520 Luca Porzio 2021-02-15 854 }
07a9ce0e702520 Luca Porzio 2021-02-15 855 }
"err" not set on this else path either.
07a9ce0e702520 Luca Porzio 2021-02-15 856
07a9ce0e702520 Luca Porzio 2021-02-15 857 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 858
07a9ce0e702520 Luca Porzio 2021-02-15 859 if (host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 860 host->cqe_ops->cqe_disable(host);
07a9ce0e702520 Luca Porzio 2021-02-15 861 host->cqe_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 862 pr_info("%s: Command Queue Engine disabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 863 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 864 }
07a9ce0e702520 Luca Porzio 2021-02-15 865
07a9ce0e702520 Luca Porzio 2021-02-15 866 host->hsq_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 867 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 868 }
07a9ce0e702520 Luca Porzio 2021-02-15 869
07a9ce0e702520 Luca Porzio 2021-02-15 @870 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 871 }
07a9ce0e702520 Luca Porzio 2021-02-15 872
07a9ce0e702520 Luca Porzio 2021-02-15 873
07a9ce0e702520 Luca Porzio 2021-02-15 874 static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
07a9ce0e702520 Luca Porzio 2021-02-15 875 {
07a9ce0e702520 Luca Porzio 2021-02-15 876 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 877
07a9ce0e702520 Luca Porzio 2021-02-15 878 return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
07a9ce0e702520 Luca Porzio 2021-02-15 879 }
07a9ce0e702520 Luca Porzio 2021-02-15 880
07a9ce0e702520 Luca Porzio 2021-02-15 881 static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
07a9ce0e702520 Luca Porzio 2021-02-15 882 const char *buf, size_t count)
07a9ce0e702520 Luca Porzio 2021-02-15 883 {
07a9ce0e702520 Luca Porzio 2021-02-15 884 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 @885 struct mmc_host *host = card->host;
^^^^^^^^^^
Dereferences "card"
07a9ce0e702520 Luca Porzio 2021-02-15 886 unsigned long enable;
07a9ce0e702520 Luca Porzio 2021-02-15 887 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 888
07a9ce0e702520 Luca Porzio 2021-02-15 @889 if (!card || kstrtoul(buf, 0, &enable))
^^^^^
Checked too late.
07a9ce0e702520 Luca Porzio 2021-02-15 890 return -EINVAL;
07a9ce0e702520 Luca Porzio 2021-02-15 891 if (!card->ext_csd.cmdq_support)
07a9ce0e702520 Luca Porzio 2021-02-15 892 return -EOPNOTSUPP;
07a9ce0e702520 Luca Porzio 2021-02-15 893
07a9ce0e702520 Luca Porzio 2021-02-15 894 enable = !!enable;
07a9ce0e702520 Luca Porzio 2021-02-15 895 if (enable == card->ext_csd.cmdq_en)
07a9ce0e702520 Luca Porzio 2021-02-15 896 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 897
07a9ce0e702520 Luca Porzio 2021-02-15 898 mmc_get_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 899 card->force_disable_cmdq = !enable;
07a9ce0e702520 Luca Porzio 2021-02-15 900 err = mmc_cmdq_setup(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 901 mmc_put_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 902
07a9ce0e702520 Luca Porzio 2021-02-15 903 if (err)
07a9ce0e702520 Luca Porzio 2021-02-15 904 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 905 else
07a9ce0e702520 Luca Porzio 2021-02-15 906 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 907 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38219 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
@ 2021-02-15 13:49 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2021-02-15 13:49 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 8357 bytes --]
Hi Luca,
url: https://github.com/0day-ci/linux/commits/Luca-Porzio/Support-temporarily-disable-of-the-CMDQ-mode/20210215-083730
base: 07f7e57c63aaa2afb4ea31edef05e08699a63a00
config: i386-randconfig-m021-20210215 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/mmc/core/mmc.c:870 mmc_cmdq_setup() error: uninitialized symbol 'err'.
drivers/mmc/core/mmc.c:889 cmdq_en_store() warn: variable dereferenced before check 'card' (see line 885)
vim +/err +870 drivers/mmc/core/mmc.c
07a9ce0e702520 Luca Porzio 2021-02-15 802 static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
07a9ce0e702520 Luca Porzio 2021-02-15 803 {
07a9ce0e702520 Luca Porzio 2021-02-15 804 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 805
07a9ce0e702520 Luca Porzio 2021-02-15 806 /* Check HW support */
07a9ce0e702520 Luca Porzio 2021-02-15 807 if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
07a9ce0e702520 Luca Porzio 2021-02-15 808 card->force_disable_cmdq = true;
07a9ce0e702520 Luca Porzio 2021-02-15 809
07a9ce0e702520 Luca Porzio 2021-02-15 810 /* Enable/Disable CMDQ mode */
07a9ce0e702520 Luca Porzio 2021-02-15 811 if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 812 err = mmc_cmdq_enable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 813 if (err && err != -EBADMSG)
07a9ce0e702520 Luca Porzio 2021-02-15 814 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 815 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 816 pr_warn("%s: Enabling CMDQ failed\n",
07a9ce0e702520 Luca Porzio 2021-02-15 817 mmc_hostname(card->host));
07a9ce0e702520 Luca Porzio 2021-02-15 818 card->ext_csd.cmdq_support = false;
07a9ce0e702520 Luca Porzio 2021-02-15 819 card->ext_csd.cmdq_depth = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 820 }
07a9ce0e702520 Luca Porzio 2021-02-15 821
07a9ce0e702520 Luca Porzio 2021-02-15 822 } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 823 err = mmc_cmdq_disable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 824 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 825 pr_warn("%s: Disabling CMDQ failed, error %d\n",
07a9ce0e702520 Luca Porzio 2021-02-15 826 mmc_hostname(card->host), err);
07a9ce0e702520 Luca Porzio 2021-02-15 827 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 828 }
07a9ce0e702520 Luca Porzio 2021-02-15 829 }
"err not set on else path"
07a9ce0e702520 Luca Porzio 2021-02-15 830
07a9ce0e702520 Luca Porzio 2021-02-15 831 /*
07a9ce0e702520 Luca Porzio 2021-02-15 832 * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
07a9ce0e702520 Luca Porzio 2021-02-15 833 * disabled for a time, so a flag is needed to indicate to re-enable the
07a9ce0e702520 Luca Porzio 2021-02-15 834 * Command Queue.
07a9ce0e702520 Luca Porzio 2021-02-15 835 */
07a9ce0e702520 Luca Porzio 2021-02-15 836 card->reenable_cmdq = card->ext_csd.cmdq_en;
07a9ce0e702520 Luca Porzio 2021-02-15 837
07a9ce0e702520 Luca Porzio 2021-02-15 838 /* Enable/Disable Host CQE */
07a9ce0e702520 Luca Porzio 2021-02-15 839 if (!card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 840
07a9ce0e702520 Luca Porzio 2021-02-15 841 if (host->cqe_ops && !host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 842 err = host->cqe_ops->cqe_enable(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 843 if (!err) {
07a9ce0e702520 Luca Porzio 2021-02-15 844 host->cqe_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 845
07a9ce0e702520 Luca Porzio 2021-02-15 846 if (card->ext_csd.cmdq_en) {
07a9ce0e702520 Luca Porzio 2021-02-15 847 pr_info("%s: Command Queue Engine enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 848 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 849 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 850 host->hsq_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 851 pr_info("%s: Host Software Queue enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 852 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 853 }
07a9ce0e702520 Luca Porzio 2021-02-15 854 }
07a9ce0e702520 Luca Porzio 2021-02-15 855 }
"err" not set on this else path either.
07a9ce0e702520 Luca Porzio 2021-02-15 856
07a9ce0e702520 Luca Porzio 2021-02-15 857 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 858
07a9ce0e702520 Luca Porzio 2021-02-15 859 if (host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 860 host->cqe_ops->cqe_disable(host);
07a9ce0e702520 Luca Porzio 2021-02-15 861 host->cqe_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 862 pr_info("%s: Command Queue Engine disabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 863 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 864 }
07a9ce0e702520 Luca Porzio 2021-02-15 865
07a9ce0e702520 Luca Porzio 2021-02-15 866 host->hsq_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 867 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 868 }
07a9ce0e702520 Luca Porzio 2021-02-15 869
07a9ce0e702520 Luca Porzio 2021-02-15 @870 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 871 }
07a9ce0e702520 Luca Porzio 2021-02-15 872
07a9ce0e702520 Luca Porzio 2021-02-15 873
07a9ce0e702520 Luca Porzio 2021-02-15 874 static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
07a9ce0e702520 Luca Porzio 2021-02-15 875 {
07a9ce0e702520 Luca Porzio 2021-02-15 876 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 877
07a9ce0e702520 Luca Porzio 2021-02-15 878 return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
07a9ce0e702520 Luca Porzio 2021-02-15 879 }
07a9ce0e702520 Luca Porzio 2021-02-15 880
07a9ce0e702520 Luca Porzio 2021-02-15 881 static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
07a9ce0e702520 Luca Porzio 2021-02-15 882 const char *buf, size_t count)
07a9ce0e702520 Luca Porzio 2021-02-15 883 {
07a9ce0e702520 Luca Porzio 2021-02-15 884 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 @885 struct mmc_host *host = card->host;
^^^^^^^^^^
Dereferences "card"
07a9ce0e702520 Luca Porzio 2021-02-15 886 unsigned long enable;
07a9ce0e702520 Luca Porzio 2021-02-15 887 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 888
07a9ce0e702520 Luca Porzio 2021-02-15 @889 if (!card || kstrtoul(buf, 0, &enable))
^^^^^
Checked too late.
07a9ce0e702520 Luca Porzio 2021-02-15 890 return -EINVAL;
07a9ce0e702520 Luca Porzio 2021-02-15 891 if (!card->ext_csd.cmdq_support)
07a9ce0e702520 Luca Porzio 2021-02-15 892 return -EOPNOTSUPP;
07a9ce0e702520 Luca Porzio 2021-02-15 893
07a9ce0e702520 Luca Porzio 2021-02-15 894 enable = !!enable;
07a9ce0e702520 Luca Porzio 2021-02-15 895 if (enable == card->ext_csd.cmdq_en)
07a9ce0e702520 Luca Porzio 2021-02-15 896 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 897
07a9ce0e702520 Luca Porzio 2021-02-15 898 mmc_get_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 899 card->force_disable_cmdq = !enable;
07a9ce0e702520 Luca Porzio 2021-02-15 900 err = mmc_cmdq_setup(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 901 mmc_put_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 902
07a9ce0e702520 Luca Porzio 2021-02-15 903 if (err)
07a9ce0e702520 Luca Porzio 2021-02-15 904 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 905 else
07a9ce0e702520 Luca Porzio 2021-02-15 906 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 907 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38219 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
2021-02-15 0:32 Luca Porzio
2021-02-15 13:49 ` Dan Carpenter
@ 2021-02-17 13:04 ` kernel test robot
2021-03-02 10:46 ` Ulf Hansson
2021-03-08 17:05 ` Ulf Hansson
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-02-17 13:04 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 9291 bytes --]
Hi Luca,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on next-20210212]
[cannot apply to linus/master mmc/mmc-next v5.11-rc7 v5.11-rc6 v5.11-rc5 v5.11]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luca-Porzio/Support-temporarily-disable-of-the-CMDQ-mode/20210215-083730
base: 07f7e57c63aaa2afb4ea31edef05e08699a63a00
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
"cppcheck warnings: (new ones prefixed by >>)"
>> drivers/mmc/core/mmc.c:870:9: warning: Uninitialized variable: err [uninitvar]
return err;
^
cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
>> drivers/mmc/core/mmc.c:885:26: warning: Either the condition '!card' is redundant or there is possible null pointer dereference: card. [nullPointerRedundantCheck]
struct mmc_host *host = card->host;
^
drivers/mmc/core/mmc.c:889:6: note: Assuming that condition '!card' is not redundant
if (!card || kstrtoul(buf, 0, &enable))
^
drivers/mmc/core/mmc.c:885:26: note: Null pointer dereference
struct mmc_host *host = card->host;
^
vim +885 drivers/mmc/core/mmc.c
07a9ce0e702520 Luca Porzio 2021-02-15 797
07a9ce0e702520 Luca Porzio 2021-02-15 798
07a9ce0e702520 Luca Porzio 2021-02-15 799 /* Setup command queue mode and CQE if underling hw supports it
07a9ce0e702520 Luca Porzio 2021-02-15 800 * and assuming force_disable_cmdq has not been set.
07a9ce0e702520 Luca Porzio 2021-02-15 801 */
07a9ce0e702520 Luca Porzio 2021-02-15 802 static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
07a9ce0e702520 Luca Porzio 2021-02-15 803 {
07a9ce0e702520 Luca Porzio 2021-02-15 804 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 805
07a9ce0e702520 Luca Porzio 2021-02-15 806 /* Check HW support */
07a9ce0e702520 Luca Porzio 2021-02-15 807 if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
07a9ce0e702520 Luca Porzio 2021-02-15 808 card->force_disable_cmdq = true;
07a9ce0e702520 Luca Porzio 2021-02-15 809
07a9ce0e702520 Luca Porzio 2021-02-15 810 /* Enable/Disable CMDQ mode */
07a9ce0e702520 Luca Porzio 2021-02-15 811 if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 812 err = mmc_cmdq_enable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 813 if (err && err != -EBADMSG)
07a9ce0e702520 Luca Porzio 2021-02-15 814 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 815 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 816 pr_warn("%s: Enabling CMDQ failed\n",
07a9ce0e702520 Luca Porzio 2021-02-15 817 mmc_hostname(card->host));
07a9ce0e702520 Luca Porzio 2021-02-15 818 card->ext_csd.cmdq_support = false;
07a9ce0e702520 Luca Porzio 2021-02-15 819 card->ext_csd.cmdq_depth = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 820 }
07a9ce0e702520 Luca Porzio 2021-02-15 821
07a9ce0e702520 Luca Porzio 2021-02-15 822 } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 823 err = mmc_cmdq_disable(card);
07a9ce0e702520 Luca Porzio 2021-02-15 824 if (err) {
07a9ce0e702520 Luca Porzio 2021-02-15 825 pr_warn("%s: Disabling CMDQ failed, error %d\n",
07a9ce0e702520 Luca Porzio 2021-02-15 826 mmc_hostname(card->host), err);
07a9ce0e702520 Luca Porzio 2021-02-15 827 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 828 }
07a9ce0e702520 Luca Porzio 2021-02-15 829 }
07a9ce0e702520 Luca Porzio 2021-02-15 830
07a9ce0e702520 Luca Porzio 2021-02-15 831 /*
07a9ce0e702520 Luca Porzio 2021-02-15 832 * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
07a9ce0e702520 Luca Porzio 2021-02-15 833 * disabled for a time, so a flag is needed to indicate to re-enable the
07a9ce0e702520 Luca Porzio 2021-02-15 834 * Command Queue.
07a9ce0e702520 Luca Porzio 2021-02-15 835 */
07a9ce0e702520 Luca Porzio 2021-02-15 836 card->reenable_cmdq = card->ext_csd.cmdq_en;
07a9ce0e702520 Luca Porzio 2021-02-15 837
07a9ce0e702520 Luca Porzio 2021-02-15 838 /* Enable/Disable Host CQE */
07a9ce0e702520 Luca Porzio 2021-02-15 839 if (!card->force_disable_cmdq) {
07a9ce0e702520 Luca Porzio 2021-02-15 840
07a9ce0e702520 Luca Porzio 2021-02-15 841 if (host->cqe_ops && !host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 842 err = host->cqe_ops->cqe_enable(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 843 if (!err) {
07a9ce0e702520 Luca Porzio 2021-02-15 844 host->cqe_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 845
07a9ce0e702520 Luca Porzio 2021-02-15 846 if (card->ext_csd.cmdq_en) {
07a9ce0e702520 Luca Porzio 2021-02-15 847 pr_info("%s: Command Queue Engine enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 848 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 849 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 850 host->hsq_enabled = true;
07a9ce0e702520 Luca Porzio 2021-02-15 851 pr_info("%s: Host Software Queue enabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 852 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 853 }
07a9ce0e702520 Luca Porzio 2021-02-15 854 }
07a9ce0e702520 Luca Porzio 2021-02-15 855 }
07a9ce0e702520 Luca Porzio 2021-02-15 856
07a9ce0e702520 Luca Porzio 2021-02-15 857 } else {
07a9ce0e702520 Luca Porzio 2021-02-15 858
07a9ce0e702520 Luca Porzio 2021-02-15 859 if (host->cqe_enabled) {
07a9ce0e702520 Luca Porzio 2021-02-15 860 host->cqe_ops->cqe_disable(host);
07a9ce0e702520 Luca Porzio 2021-02-15 861 host->cqe_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 862 pr_info("%s: Command Queue Engine disabled\n",
07a9ce0e702520 Luca Porzio 2021-02-15 863 mmc_hostname(host));
07a9ce0e702520 Luca Porzio 2021-02-15 864 }
07a9ce0e702520 Luca Porzio 2021-02-15 865
07a9ce0e702520 Luca Porzio 2021-02-15 866 host->hsq_enabled = false;
07a9ce0e702520 Luca Porzio 2021-02-15 867 err = 0;
07a9ce0e702520 Luca Porzio 2021-02-15 868 }
07a9ce0e702520 Luca Porzio 2021-02-15 869
07a9ce0e702520 Luca Porzio 2021-02-15 @870 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 871 }
07a9ce0e702520 Luca Porzio 2021-02-15 872
07a9ce0e702520 Luca Porzio 2021-02-15 873
07a9ce0e702520 Luca Porzio 2021-02-15 874 static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
07a9ce0e702520 Luca Porzio 2021-02-15 875 {
07a9ce0e702520 Luca Porzio 2021-02-15 876 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 877
07a9ce0e702520 Luca Porzio 2021-02-15 878 return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
07a9ce0e702520 Luca Porzio 2021-02-15 879 }
07a9ce0e702520 Luca Porzio 2021-02-15 880
07a9ce0e702520 Luca Porzio 2021-02-15 881 static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
07a9ce0e702520 Luca Porzio 2021-02-15 882 const char *buf, size_t count)
07a9ce0e702520 Luca Porzio 2021-02-15 883 {
07a9ce0e702520 Luca Porzio 2021-02-15 884 struct mmc_card *card = mmc_dev_to_card(dev);
07a9ce0e702520 Luca Porzio 2021-02-15 @885 struct mmc_host *host = card->host;
07a9ce0e702520 Luca Porzio 2021-02-15 886 unsigned long enable;
07a9ce0e702520 Luca Porzio 2021-02-15 887 int err;
07a9ce0e702520 Luca Porzio 2021-02-15 888
07a9ce0e702520 Luca Porzio 2021-02-15 889 if (!card || kstrtoul(buf, 0, &enable))
07a9ce0e702520 Luca Porzio 2021-02-15 890 return -EINVAL;
07a9ce0e702520 Luca Porzio 2021-02-15 891 if (!card->ext_csd.cmdq_support)
07a9ce0e702520 Luca Porzio 2021-02-15 892 return -EOPNOTSUPP;
07a9ce0e702520 Luca Porzio 2021-02-15 893
07a9ce0e702520 Luca Porzio 2021-02-15 894 enable = !!enable;
07a9ce0e702520 Luca Porzio 2021-02-15 895 if (enable == card->ext_csd.cmdq_en)
07a9ce0e702520 Luca Porzio 2021-02-15 896 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 897
07a9ce0e702520 Luca Porzio 2021-02-15 898 mmc_get_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 899 card->force_disable_cmdq = !enable;
07a9ce0e702520 Luca Porzio 2021-02-15 900 err = mmc_cmdq_setup(host, card);
07a9ce0e702520 Luca Porzio 2021-02-15 901 mmc_put_card(card, NULL);
07a9ce0e702520 Luca Porzio 2021-02-15 902
07a9ce0e702520 Luca Porzio 2021-02-15 903 if (err)
07a9ce0e702520 Luca Porzio 2021-02-15 904 return err;
07a9ce0e702520 Luca Porzio 2021-02-15 905 else
07a9ce0e702520 Luca Porzio 2021-02-15 906 return count;
07a9ce0e702520 Luca Porzio 2021-02-15 907 }
07a9ce0e702520 Luca Porzio 2021-02-15 908
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
2021-02-15 0:32 Luca Porzio
2021-02-15 13:49 ` Dan Carpenter
2021-02-17 13:04 ` kernel test robot
@ 2021-03-02 10:46 ` Ulf Hansson
2021-03-10 8:54 ` Adrian Hunter
2021-03-08 17:05 ` Ulf Hansson
3 siblings, 1 reply; 10+ messages in thread
From: Ulf Hansson @ 2021-03-02 10:46 UTC (permalink / raw)
To: Luca Porzio, Adrian Hunter; +Cc: linux-mmc, Zhan Liu, Luca Porzio
+ Adrian
On Mon, 15 Feb 2021 at 01:33, Luca Porzio <porzio@gmail.com> wrote:
>
> cmdq_en attribute in sysfs now can now be written.
> When 0 is written:
> CMDQ is disabled and kept disabled across device reboots.
> When 1 is written:
> CMDQ mode is instantly reneabled (if supported).
>
> Signed-off-by: Luca Porzio <lporzio@micron.com>
> Signed-off-by: Zhan Liu <zliua@micron.com>
Luca, thanks for your patch! I am about to start to review this.
I have also looped in Adrian to get his opinions.
Get back to you soon!
Kind regards
Uffe
> ---
> drivers/mmc/core/mmc.c | 152 ++++++++++++++++++++++++++++++---------
> include/linux/mmc/card.h | 1 +
> 2 files changed, 118 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 0d80b72ddde8..5c7d5bac5c00 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -794,7 +794,120 @@ MMC_DEV_ATTR(enhanced_rpmb_supported, "%#x\n",
> MMC_DEV_ATTR(rel_sectors, "%#x\n", card->ext_csd.rel_sectors);
> MMC_DEV_ATTR(ocr, "0x%08x\n", card->ocr);
> MMC_DEV_ATTR(rca, "0x%04x\n", card->rca);
> -MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en);
> +
> +
> +/* Setup command queue mode and CQE if underling hw supports it
> + * and assuming force_disable_cmdq has not been set.
> + */
> +static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
> +{
> + int err;
> +
> + /* Check HW support */
> + if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
> + card->force_disable_cmdq = true;
> +
> + /* Enable/Disable CMDQ mode */
> + if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
> + err = mmc_cmdq_enable(card);
> + if (err && err != -EBADMSG)
> + return err;
> + if (err) {
> + pr_warn("%s: Enabling CMDQ failed\n",
> + mmc_hostname(card->host));
> + card->ext_csd.cmdq_support = false;
> + card->ext_csd.cmdq_depth = 0;
> + }
> +
> + } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
> + err = mmc_cmdq_disable(card);
> + if (err) {
> + pr_warn("%s: Disabling CMDQ failed, error %d\n",
> + mmc_hostname(card->host), err);
> + err = 0;
> + }
> + }
> +
> + /*
> + * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
> + * disabled for a time, so a flag is needed to indicate to re-enable the
> + * Command Queue.
> + */
> + card->reenable_cmdq = card->ext_csd.cmdq_en;
> +
> + /* Enable/Disable Host CQE */
> + if (!card->force_disable_cmdq) {
> +
> + if (host->cqe_ops && !host->cqe_enabled) {
> + err = host->cqe_ops->cqe_enable(host, card);
> + if (!err) {
> + host->cqe_enabled = true;
> +
> + if (card->ext_csd.cmdq_en) {
> + pr_info("%s: Command Queue Engine enabled\n",
> + mmc_hostname(host));
> + } else {
> + host->hsq_enabled = true;
> + pr_info("%s: Host Software Queue enabled\n",
> + mmc_hostname(host));
> + }
> + }
> + }
> +
> + } else {
> +
> + if (host->cqe_enabled) {
> + host->cqe_ops->cqe_disable(host);
> + host->cqe_enabled = false;
> + pr_info("%s: Command Queue Engine disabled\n",
> + mmc_hostname(host));
> + }
> +
> + host->hsq_enabled = false;
> + err = 0;
> + }
> +
> + return err;
> +}
> +
> +
> +static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct mmc_card *card = mmc_dev_to_card(dev);
> +
> + return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
> +}
> +
> +static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct mmc_card *card = mmc_dev_to_card(dev);
> + struct mmc_host *host = card->host;
> + unsigned long enable;
> + int err;
> +
> + if (!card || kstrtoul(buf, 0, &enable))
> + return -EINVAL;
> + if (!card->ext_csd.cmdq_support)
> + return -EOPNOTSUPP;
> +
> + enable = !!enable;
> + if (enable == card->ext_csd.cmdq_en)
> + return count;
> +
> + mmc_get_card(card, NULL);
> + card->force_disable_cmdq = !enable;
> + err = mmc_cmdq_setup(host, card);
> + mmc_put_card(card, NULL);
> +
> + if (err)
> + return err;
> + else
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(cmdq_en);
> +
>
> static ssize_t mmc_fwrev_show(struct device *dev,
> struct device_attribute *attr,
> @@ -1838,40 +1951,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> * Enable Command Queue if supported. Note that Packed Commands cannot
> * be used with Command Queue.
> */
> - card->ext_csd.cmdq_en = false;
> - if (card->ext_csd.cmdq_support && host->caps2 & MMC_CAP2_CQE) {
> - err = mmc_cmdq_enable(card);
> - if (err && err != -EBADMSG)
> - goto free_card;
> - if (err) {
> - pr_warn("%s: Enabling CMDQ failed\n",
> - mmc_hostname(card->host));
> - card->ext_csd.cmdq_support = false;
> - card->ext_csd.cmdq_depth = 0;
> - }
> - }
> - /*
> - * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
> - * disabled for a time, so a flag is needed to indicate to re-enable the
> - * Command Queue.
> - */
> - card->reenable_cmdq = card->ext_csd.cmdq_en;
> -
> - if (host->cqe_ops && !host->cqe_enabled) {
> - err = host->cqe_ops->cqe_enable(host, card);
> - if (!err) {
> - host->cqe_enabled = true;
> -
> - if (card->ext_csd.cmdq_en) {
> - pr_info("%s: Command Queue Engine enabled\n",
> - mmc_hostname(host));
> - } else {
> - host->hsq_enabled = true;
> - pr_info("%s: Host Software Queue enabled\n",
> - mmc_hostname(host));
> - }
> - }
> - }
> + err = mmc_cmdq_setup(host, card);
> + if (err)
> + goto free_card;
>
> if (host->caps2 & MMC_CAP2_AVOID_3_3V &&
> host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index f9ad35dd6012..e554bb0cf722 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -272,6 +272,7 @@ struct mmc_card {
> #define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */
>
> bool reenable_cmdq; /* Re-enable Command Queue */
> + bool force_disable_cmdq; /* Keep Command Queue disabled */
>
> unsigned int erase_size; /* erase size in sectors */
> unsigned int erase_shift; /* if erase unit is power 2 */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
2021-03-02 10:46 ` Ulf Hansson
@ 2021-03-10 8:54 ` Adrian Hunter
0 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2021-03-10 8:54 UTC (permalink / raw)
To: Ulf Hansson, Luca Porzio; +Cc: linux-mmc, Zhan Liu, Luca Porzio
On 2/03/21 12:46 pm, Ulf Hansson wrote:
> + Adrian
>
> On Mon, 15 Feb 2021 at 01:33, Luca Porzio <porzio@gmail.com> wrote:
>>
>> cmdq_en attribute in sysfs now can now be written.
>> When 0 is written:
>> CMDQ is disabled and kept disabled across device reboots.
>> When 1 is written:
>> CMDQ mode is instantly reneabled (if supported).
>>
>> Signed-off-by: Luca Porzio <lporzio@micron.com>
>> Signed-off-by: Zhan Liu <zliua@micron.com>
>
> Luca, thanks for your patch! I am about to start to review this.
>
> I have also looped in Adrian to get his opinions.
>
> Get back to you soon!
>
> Kind regards
> Uffe
>
>> ---
>> drivers/mmc/core/mmc.c | 152 ++++++++++++++++++++++++++++++---------
>> include/linux/mmc/card.h | 1 +
>> 2 files changed, 118 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 0d80b72ddde8..5c7d5bac5c00 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -794,7 +794,120 @@ MMC_DEV_ATTR(enhanced_rpmb_supported, "%#x\n",
>> MMC_DEV_ATTR(rel_sectors, "%#x\n", card->ext_csd.rel_sectors);
>> MMC_DEV_ATTR(ocr, "0x%08x\n", card->ocr);
>> MMC_DEV_ATTR(rca, "0x%04x\n", card->rca);
>> -MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en);
>> +
>> +
>> +/* Setup command queue mode and CQE if underling hw supports it
>> + * and assuming force_disable_cmdq has not been set.
>> + */
>> +static int mmc_cmdq_setup(struct mmc_host *host, struct mmc_card *card)
>> +{
>> + int err;
>> +
>> + /* Check HW support */
>> + if (!card->ext_csd.cmdq_support || !(host->caps2 & MMC_CAP2_CQE))
>> + card->force_disable_cmdq = true;
>> +
>> + /* Enable/Disable CMDQ mode */
>> + if (!card->ext_csd.cmdq_en && !card->force_disable_cmdq) {
>> + err = mmc_cmdq_enable(card);
>> + if (err && err != -EBADMSG)
>> + return err;
>> + if (err) {
>> + pr_warn("%s: Enabling CMDQ failed\n",
>> + mmc_hostname(card->host));
>> + card->ext_csd.cmdq_support = false;
>> + card->ext_csd.cmdq_depth = 0;
>> + }
>> +
>> + } else if (card->ext_csd.cmdq_en && card->force_disable_cmdq) {
>> + err = mmc_cmdq_disable(card);
>> + if (err) {
>> + pr_warn("%s: Disabling CMDQ failed, error %d\n",
>> + mmc_hostname(card->host), err);
>> + err = 0;
>> + }
>> + }
>> +
>> + /*
>> + * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
>> + * disabled for a time, so a flag is needed to indicate to re-enable the
>> + * Command Queue.
>> + */
>> + card->reenable_cmdq = card->ext_csd.cmdq_en;
>> +
>> + /* Enable/Disable Host CQE */
>> + if (!card->force_disable_cmdq) {
>> +
>> + if (host->cqe_ops && !host->cqe_enabled) {
>> + err = host->cqe_ops->cqe_enable(host, card);
>> + if (!err) {
>> + host->cqe_enabled = true;
Re-initializing the card is also a recovery path for the block driver.
Changing host->cqe_enabled during a recovery reset, creates an unexpected
dependency for the block driver. That should not be necessary, and given
that cqhci does memory allocation as part of enabling, it is better not to
disable / re-enable if it can be helped.
From a design point of view, it is really the block driver that should
control the use of command queuing rather than expecting it to cope with
changes from a lower level.
>> +
>> + if (card->ext_csd.cmdq_en) {
>> + pr_info("%s: Command Queue Engine enabled\n",
>> + mmc_hostname(host));
>> + } else {
>> + host->hsq_enabled = true;
>> + pr_info("%s: Host Software Queue enabled\n",
>> + mmc_hostname(host));
>> + }
>> + }
>> + }
>> +
>> + } else {
>> +
>> + if (host->cqe_enabled) {
>> + host->cqe_ops->cqe_disable(host);
>> + host->cqe_enabled = false;
>> + pr_info("%s: Command Queue Engine disabled\n",
>> + mmc_hostname(host));
>> + }
>> +
>> + host->hsq_enabled = false;
This looks quite wrong for hsq which is currently not used with
cmdq.
>> + err = 0;
>> + }
>> +
>> + return err;
>> +}
>> +
>> +
>> +static ssize_t cmdq_en_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> + struct mmc_card *card = mmc_dev_to_card(dev);
>> +
>> + return sprintf(buf, "%d\n", card->ext_csd.cmdq_en);
>> +}
>> +
>> +static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct mmc_card *card = mmc_dev_to_card(dev);
>> + struct mmc_host *host = card->host;
>> + unsigned long enable;
>> + int err;
>> +
>> + if (!card || kstrtoul(buf, 0, &enable))
>> + return -EINVAL;
>> + if (!card->ext_csd.cmdq_support)
>> + return -EOPNOTSUPP;
>> +
>> + enable = !!enable;
>> + if (enable == card->ext_csd.cmdq_en)
>> + return count;
>> +
>> + mmc_get_card(card, NULL);
>> + card->force_disable_cmdq = !enable;
>> + err = mmc_cmdq_setup(host, card);
>> + mmc_put_card(card, NULL);
>> +
>> + if (err)
>> + return err;
>> + else
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR_RW(cmdq_en);
>> +
>>
>> static ssize_t mmc_fwrev_show(struct device *dev,
>> struct device_attribute *attr,
>> @@ -1838,40 +1951,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>> * Enable Command Queue if supported. Note that Packed Commands cannot
>> * be used with Command Queue.
>> */
>> - card->ext_csd.cmdq_en = false;
>> - if (card->ext_csd.cmdq_support && host->caps2 & MMC_CAP2_CQE) {
>> - err = mmc_cmdq_enable(card);
>> - if (err && err != -EBADMSG)
>> - goto free_card;
>> - if (err) {
>> - pr_warn("%s: Enabling CMDQ failed\n",
>> - mmc_hostname(card->host));
>> - card->ext_csd.cmdq_support = false;
>> - card->ext_csd.cmdq_depth = 0;
>> - }
>> - }
>> - /*
>> - * In some cases (e.g. RPMB or mmc_test), the Command Queue must be
>> - * disabled for a time, so a flag is needed to indicate to re-enable the
>> - * Command Queue.
>> - */
>> - card->reenable_cmdq = card->ext_csd.cmdq_en;
>> -
>> - if (host->cqe_ops && !host->cqe_enabled) {
>> - err = host->cqe_ops->cqe_enable(host, card);
>> - if (!err) {
>> - host->cqe_enabled = true;
>> -
>> - if (card->ext_csd.cmdq_en) {
>> - pr_info("%s: Command Queue Engine enabled\n",
>> - mmc_hostname(host));
>> - } else {
>> - host->hsq_enabled = true;
>> - pr_info("%s: Host Software Queue enabled\n",
>> - mmc_hostname(host));
>> - }
>> - }
>> - }
>> + err = mmc_cmdq_setup(host, card);
>> + if (err)
>> + goto free_card;
>>
>> if (host->caps2 & MMC_CAP2_AVOID_3_3V &&
>> host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index f9ad35dd6012..e554bb0cf722 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -272,6 +272,7 @@ struct mmc_card {
>> #define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */
>>
>> bool reenable_cmdq; /* Re-enable Command Queue */
>> + bool force_disable_cmdq; /* Keep Command Queue disabled */
>>
>> unsigned int erase_size; /* erase size in sectors */
>> unsigned int erase_shift; /* if erase unit is power 2 */
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable
2021-02-15 0:32 Luca Porzio
` (2 preceding siblings ...)
2021-03-02 10:46 ` Ulf Hansson
@ 2021-03-08 17:05 ` Ulf Hansson
[not found] ` <CABhGgDPpUXPHJ49E_ku5N-fO=GWZKTdQQUGugAruG6y2=J1YgA@mail.gmail.com>
3 siblings, 1 reply; 10+ messages in thread
From: Ulf Hansson @ 2021-03-08 17:05 UTC (permalink / raw)
To: Luca Porzio; +Cc: linux-mmc, Zhan Liu, Luca Porzio
On Mon, 15 Feb 2021 at 01:33, Luca Porzio <porzio@gmail.com> wrote:
>
> cmdq_en attribute in sysfs now can now be written.
> When 0 is written:
> CMDQ is disabled and kept disabled across device reboots.
> When 1 is written:
> CMDQ mode is instantly reneabled (if supported).
>
> Signed-off-by: Luca Porzio <lporzio@micron.com>
> Signed-off-by: Zhan Liu <zliua@micron.com>
> ---
> drivers/mmc/core/mmc.c | 152 ++++++++++++++++++++++++++++++---------
> include/linux/mmc/card.h | 1 +
> 2 files changed, 118 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
[...]
> +static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct mmc_card *card = mmc_dev_to_card(dev);
> + struct mmc_host *host = card->host;
> + unsigned long enable;
> + int err;
> +
> + if (!card || kstrtoul(buf, 0, &enable))
> + return -EINVAL;
> + if (!card->ext_csd.cmdq_support)
> + return -EOPNOTSUPP;
> +
> + enable = !!enable;
> + if (enable == card->ext_csd.cmdq_en)
> + return count;
> +
> + mmc_get_card(card, NULL);
This means adding a new path for when the host needs to get locked
(claimed), which is the opposite direction of what we have been
working on for SD/eMMC during the last couple of years.
Please have a look at mmc_blk_issue_drv_op(), where you can find how
these kinds of requests are being funneled through the mmc block
device layer instead. This is the preferred option.
That said, I am actually wondering if we perhaps could manage the
enable/disable of CQE "automagically" for FFU, along the lines of what
we do for RPMB already. In fact, maybe the best/easiest approach is to
*always* disable CQE when there is a request being received through
the mmc ioctl interface. Of course, then if we disabled CQE we should
also re-enable it when the ioctl request has been completed.
What do you think?
> + card->force_disable_cmdq = !enable;
> + err = mmc_cmdq_setup(host, card);
> + mmc_put_card(card, NULL);
> +
> + if (err)
> + return err;
> + else
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(cmdq_en);
> +
>
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-10 8:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 3:09 [RFC PATCH 2/2] Make cmdq_en attribute writeable kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-02-15 2:21 kernel test robot
2021-02-15 0:32 Luca Porzio
2021-02-15 13:49 ` Dan Carpenter
2021-02-15 13:49 ` Dan Carpenter
2021-02-17 13:04 ` kernel test robot
2021-03-02 10:46 ` Ulf Hansson
2021-03-10 8:54 ` Adrian Hunter
2021-03-08 17:05 ` Ulf Hansson
[not found] ` <CABhGgDPpUXPHJ49E_ku5N-fO=GWZKTdQQUGugAruG6y2=J1YgA@mail.gmail.com>
2021-03-09 15:47 ` Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.