| Message ID | 20230401165723.19762-2-quic_sartgarg@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce new vendor op and export few symbols | expand |
On Sat, Apr 01, 2023 at 10:27:22PM +0530, Sarthak Garg wrote: > Define new ops to let vendor enable internal features in > mmc_suspend/resume paths like partial init feature. 1) vendors have absolutely no business doing anything, you might be doing either something entirely wrong or use the wrong terminology here. 2) any kind of core hook not only needs a very good description, but also an actual user that goes along in the same series.
You don't get it. There is no such thing "as vendor files". I'm not sure where you get your terminology from, but whatever that is might be your source of the fundamental misunderstanding how Linux kernel development works. I would recommend to take some training before wasting everyones time.
Sorry for the confusion by vendor file I meant driver file for Qualcomm SDCC controller (sdhci-msm.c). Further to make things more clear I will push the complete series. > -----Original Message----- > From: Christoph Hellwig <hch@infradead.org> > Sent: Friday, April 14, 2023 11:06 AM > To: Sarthak Garg (QUIC) <quic_sartgarg@quicinc.com> > Cc: Christoph Hellwig <hch@infradead.org>; adrian.hunter@intel.com; > ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; Ram Prakash Gupta > (QUIC) <quic_rampraka@quicinc.com>; Bhaskar Valaboju (QUIC) > <quic_bhaskarv@quicinc.com>; Sachin Gupta (QUIC) > <quic_sachgupt@quicinc.com>; Pradeep Pragallapati (QUIC) > <quic_pragalla@quicinc.com>; Sayali Lokhande (QUIC) > <quic_sayalil@quicinc.com>; Brian Norris <briannorris@chromium.org>; > Wolfram Sang <wsa+renesas@sang-engineering.com>; Linus Walleij > <linus.walleij@linaro.org> > Subject: Re: [PATCH V1 1/2] mmc: core: Define new vendor ops to enable > internal features > > You don't get it. There is no such thing "as vendor files". > > I'm not sure where you get your terminology from, but whatever that is might > be your source of the fundamental misunderstanding how Linux kernel > development works. I would recommend to take some training before wasting > everyones time.
On Fri, Apr 14, 2023 at 06:52:18AM +0000, Sarthak Garg (QUIC) wrote:
> Sorry for the confusion by vendor file I meant driver file for Qualcomm SDCC controller (sdhci-msm.c).
This is still not how we do development. The two series you've been
pointed out got valuable feedback that;s been ignored for between one
and four years, that needs to be followed up with.
You're not going to get magic hooks for your driver that you're not
sharing with us just because you're too lazy to follow up on the review
comments.
Thanks for your valuable comments. We didn't ignore the previous comments instead we tried to address most of the comments by trying the suggested alternatives as well but didn't see power improvement as compared to this feature. Moreover we got the intuition that maintainability was the main concern hence we came up with this newer approach of hooks to limit the lines of code in core layer. Every change was pushed earlier in the previous posts and this time we just refactored the code and was about to push the series but as per current discussion we'll be reviving the old discussion and try to close all the comments. Closing this thread now. > -----Original Message----- > From: Christoph Hellwig <hch@infradead.org> > Sent: Friday, April 14, 2023 6:46 PM > To: Sarthak Garg (QUIC) <quic_sartgarg@quicinc.com> > Cc: Christoph Hellwig <hch@infradead.org>; adrian.hunter@intel.com; > ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; Ram Prakash Gupta > (QUIC) <quic_rampraka@quicinc.com>; Bhaskar Valaboju (QUIC) > <quic_bhaskarv@quicinc.com>; Sachin Gupta (QUIC) > <quic_sachgupt@quicinc.com>; Pradeep Pragallapati (QUIC) > <quic_pragalla@quicinc.com>; Sayali Lokhande (QUIC) > <quic_sayalil@quicinc.com>; Brian Norris <briannorris@chromium.org>; > Wolfram Sang <wsa+renesas@sang-engineering.com>; Linus Walleij > <linus.walleij@linaro.org> > Subject: Re: [PATCH V1 1/2] mmc: core: Define new vendor ops to enable > internal features > > On Fri, Apr 14, 2023 at 06:52:18AM +0000, Sarthak Garg (QUIC) wrote: > > Sorry for the confusion by vendor file I meant driver file for Qualcomm SDCC > controller (sdhci-msm.c). > > This is still not how we do development. The two series you've been pointed out > got valuable feedback that;s been ignored for between one and four years, that > needs to be followed up with. > > You're not going to get magic hooks for your driver that you're not sharing with > us just because you're too lazy to follow up on the review comments.
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 89cd48fcec79..32386e4644df 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -2112,9 +2112,11 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend || (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND))) err = mmc_poweroff_notify(host->card, notify_type); - else if (mmc_can_sleep(host->card)) + else if (mmc_can_sleep(host->card)) { + if (host->ops->cache_card_properties) + host->ops->cache_card_properties(host); err = mmc_sleep(host); - else if (!mmc_host_is_spi(host)) + } else if (!mmc_host_is_spi(host)) err = mmc_deselect_cards(host); if (!err) { @@ -2149,6 +2151,7 @@ static int mmc_suspend(struct mmc_host *host) static int _mmc_resume(struct mmc_host *host) { int err = 0; + bool partial_init_success = false; mmc_claim_host(host); @@ -2156,7 +2159,11 @@ static int _mmc_resume(struct mmc_host *host) goto out; mmc_power_up(host, host->card->ocr); - err = mmc_init_card(host, host->card->ocr, host->card); + + if (host->ops->partial_init_card) + partial_init_success = host->ops->partial_init_card(host); + if (!partial_init_success) + err = mmc_init_card(host, host->card->ocr, host->card); mmc_card_clr_suspended(host->card); out: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 461d1543893b..0a796a34b83d 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -212,6 +212,10 @@ struct mmc_host_ops { /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP. */ int (*init_sd_express)(struct mmc_host *host, struct mmc_ios *ios); + + void (*cache_card_properties)(struct mmc_host *host); + bool (*partial_init_card)(struct mmc_host *host); + }; struct mmc_cqe_ops {
Define new ops to let vendor enable internal features in mmc_suspend/resume paths like partial init feature. Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> --- drivers/mmc/core/mmc.c | 13 ++++++++++--- include/linux/mmc/host.h | 4 ++++ 2 files changed, 14 insertions(+), 3 deletions(-)