| Message ID | 20201106120933.7190-2-muhammad.husaini.zulkifli@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mmc: sdhci-of-arasan: Specify .clk_ops for Keem Bay SOC | expand |
On Fri, 6 Nov 2020 at 05:10, <muhammad.husaini.zulkifli@intel.com> wrote: > > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations handling") > introduces platform specific SDHCI clock operation. > > This patch declares the clock operation for Keem Bay. > Add clk_ops for SD, EMMC and SDIO operations. The above commit message doesn't really tell why or what goes on here. Can please try to clarify that. > > Fixes: 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for Intel Keem Bay") Is $subject patch fixing a bug/regression? Kind regards Uffe > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-of-arasan.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index 829ccef87426..012d52e1abee 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -1199,16 +1199,19 @@ static struct sdhci_arasan_of_data sdhci_arasan_versal_data = { > static struct sdhci_arasan_of_data intel_keembay_emmc_data = { > .soc_ctl_map = &intel_keembay_soc_ctl_map, > .pdata = &sdhci_keembay_emmc_pdata, > + .clk_ops = &arasan_clk_ops, > }; > > static struct sdhci_arasan_of_data intel_keembay_sd_data = { > .soc_ctl_map = &intel_keembay_soc_ctl_map, > .pdata = &sdhci_keembay_sd_pdata, > + .clk_ops = &arasan_clk_ops, > }; > > static struct sdhci_arasan_of_data intel_keembay_sdio_data = { > .soc_ctl_map = &intel_keembay_soc_ctl_map, > .pdata = &sdhci_keembay_sdio_pdata, > + .clk_ops = &arasan_clk_ops, > }; > > static const struct of_device_id sdhci_arasan_of_match[] = { > -- > 2.17.1 >
Hi Ulf, Thanks for your review comments. I replied inline >-----Original Message----- >From: Ulf Hansson <ulf.hansson@linaro.org> >Sent: Monday, November 16, 2020 7:41 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; Raja >Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; Wan >Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box ><david.e.box@linux.intel.com> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops for Keem >Bay SOC > >On Fri, 6 Nov 2020 at 05:10, <muhammad.husaini.zulkifli@intel.com> wrote: >> >> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >> >> Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations >> handling") introduces platform specific SDHCI clock operation. >> >> This patch declares the clock operation for Keem Bay. >> Add clk_ops for SD, EMMC and SDIO operations. > >The above commit message doesn't really tell why or what goes on here. >Can please try to clarify that. We missed out the clock operation handling patch (Commit 16ada730a759) and did not handle the clk_ops for Keem Bay SOC devices. These 2 patches (Commit 16ada730a759) and (Commit 36c6aadaae86) are merged at around the same time. We catch the issue later when trying to boot into v5.10-rc1 with Keem Bay EVM. That is why I created this patch to handle the clk_ops for Keem Bay devices. > >> >> Fixes: 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for Intel Keem >> Bay") > >Is $subject patch fixing a bug/regression? This is to fix issue on previous commit. It is a bug fix. > >Kind regards >Uffe > >> >> Signed-off-by: Muhammad Husaini Zulkifli >> <muhammad.husaini.zulkifli@intel.com> >> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> drivers/mmc/host/sdhci-of-arasan.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c >> b/drivers/mmc/host/sdhci-of-arasan.c >> index 829ccef87426..012d52e1abee 100644 >> --- a/drivers/mmc/host/sdhci-of-arasan.c >> +++ b/drivers/mmc/host/sdhci-of-arasan.c >> @@ -1199,16 +1199,19 @@ static struct sdhci_arasan_of_data >> sdhci_arasan_versal_data = { static struct sdhci_arasan_of_data >intel_keembay_emmc_data = { >> .soc_ctl_map = &intel_keembay_soc_ctl_map, >> .pdata = &sdhci_keembay_emmc_pdata, >> + .clk_ops = &arasan_clk_ops, >> }; >> >> static struct sdhci_arasan_of_data intel_keembay_sd_data = { >> .soc_ctl_map = &intel_keembay_soc_ctl_map, >> .pdata = &sdhci_keembay_sd_pdata, >> + .clk_ops = &arasan_clk_ops, >> }; >> >> static struct sdhci_arasan_of_data intel_keembay_sdio_data = { >> .soc_ctl_map = &intel_keembay_soc_ctl_map, >> .pdata = &sdhci_keembay_sdio_pdata, >> + .clk_ops = &arasan_clk_ops, >> }; >> >> static const struct of_device_id sdhci_arasan_of_match[] = { >> -- >> 2.17.1 >>
On Mon, 16 Nov 2020 at 15:22, Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> wrote: > > Hi Ulf, > > Thanks for your review comments. I replied inline > > >-----Original Message----- > >From: Ulf Hansson <ulf.hansson@linaro.org> > >Sent: Monday, November 16, 2020 7:41 PM > >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> > >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- > >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; Raja > >Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; Wan > >Mohamad, Wan Ahmad Zainie > ><wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box > ><david.e.box@linux.intel.com> > >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops for Keem > >Bay SOC > > > >On Fri, 6 Nov 2020 at 05:10, <muhammad.husaini.zulkifli@intel.com> wrote: > >> > >> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > >> > >> Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations > >> handling") introduces platform specific SDHCI clock operation. > >> > >> This patch declares the clock operation for Keem Bay. > >> Add clk_ops for SD, EMMC and SDIO operations. > > > >The above commit message doesn't really tell why or what goes on here. > >Can please try to clarify that. > > We missed out the clock operation handling patch (Commit 16ada730a759) > and did not handle the clk_ops for Keem Bay SOC devices. > These 2 patches (Commit 16ada730a759) and (Commit 36c6aadaae86) are merged at around the same time. > We catch the issue later when trying to boot into v5.10-rc1 with Keem Bay EVM. > That is why I created this patch to handle the clk_ops for Keem Bay devices. Alright, so the previous changes were simply not sufficient to complete the support. > > > > >> > >> Fixes: 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for Intel Keem > >> Bay") > > > >Is $subject patch fixing a bug/regression? > > This is to fix issue on previous commit. It is a bug fix. Has it ever worked? Did the commit you point to with the fixes tag, break some other existing functionality? [...] Kind regards Uffe
Hi Ulf, >-----Original Message----- >From: Ulf Hansson <ulf.hansson@linaro.org> >Sent: Tuesday, November 17, 2020 7:27 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; Raja >Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; Wan >Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box ><david.e.box@linux.intel.com> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops for Keem Bay >SOC > >On Mon, 16 Nov 2020 at 15:22, Zulkifli, Muhammad Husaini ><muhammad.husaini.zulkifli@intel.com> wrote: >> >> Hi Ulf, >> >> Thanks for your review comments. I replied inline >> >> >-----Original Message----- >> >From: Ulf Hansson <ulf.hansson@linaro.org> >> >Sent: Monday, November 16, 2020 7:41 PM >> >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- >> >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >> >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; >> >Raja Subramanian, Lakshmi Bai >> ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad >> >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box >> ><david.e.box@linux.intel.com> >> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops >> >for Keem Bay SOC >> > >> >On Fri, 6 Nov 2020 at 05:10, <muhammad.husaini.zulkifli@intel.com> wrote: >> >> >> >> From: Muhammad Husaini Zulkifli >> >> <muhammad.husaini.zulkifli@intel.com> >> >> >> >> Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations >> >> handling") introduces platform specific SDHCI clock operation. >> >> >> >> This patch declares the clock operation for Keem Bay. >> >> Add clk_ops for SD, EMMC and SDIO operations. >> > >> >The above commit message doesn't really tell why or what goes on here. >> >Can please try to clarify that. >> >> We missed out the clock operation handling patch (Commit 16ada730a759) >> and did not handle the clk_ops for Keem Bay SOC devices. >> These 2 patches (Commit 16ada730a759) and (Commit 36c6aadaae86) are >merged at around the same time. >> We catch the issue later when trying to boot into v5.10-rc1 with Keem Bay >EVM. >> That is why I created this patch to handle the clk_ops for Keem Bay devices. > >Alright, so the previous changes were simply not sufficient to complete the >support. > >> >> > >> >> >> >> Fixes: 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for Intel >> >> Keem >> >> Bay") >> > >> >Is $subject patch fixing a bug/regression? >> >> This is to fix issue on previous commit. It is a bug fix. > >Has it ever worked? Yes it is working. Tested with Keem Bay EVM. SD/EMMC/SDIO devices registered successfully with this fix. > >Did the commit you point to with the fixes tag, break some other existing >functionality? It should not break other existing functionality. This is specific for Keem Bay SOC only. > >[...] > >Kind regards >Uffe
On Tue, 17 Nov 2020 at 14:46, Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> wrote: > > Hi Ulf, > > >-----Original Message----- > >From: Ulf Hansson <ulf.hansson@linaro.org> > >Sent: Tuesday, November 17, 2020 7:27 PM > >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> > >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- > >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; Raja > >Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; Wan > >Mohamad, Wan Ahmad Zainie > ><wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box > ><david.e.box@linux.intel.com> > >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops for Keem Bay > >SOC > > > >On Mon, 16 Nov 2020 at 15:22, Zulkifli, Muhammad Husaini > ><muhammad.husaini.zulkifli@intel.com> wrote: > >> > >> Hi Ulf, > >> > >> Thanks for your review comments. I replied inline > >> > >> >-----Original Message----- > >> >From: Ulf Hansson <ulf.hansson@linaro.org> > >> >Sent: Monday, November 16, 2020 7:41 PM > >> >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> > >> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- > >> >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > >> >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; > >> >Raja Subramanian, Lakshmi Bai > >> ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad > >> >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box > >> ><david.e.box@linux.intel.com> > >> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops > >> >for Keem Bay SOC > >> > > >> >On Fri, 6 Nov 2020 at 05:10, <muhammad.husaini.zulkifli@intel.com> wrote: > >> >> > >> >> From: Muhammad Husaini Zulkifli > >> >> <muhammad.husaini.zulkifli@intel.com> > >> >> > >> >> Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations > >> >> handling") introduces platform specific SDHCI clock operation. > >> >> > >> >> This patch declares the clock operation for Keem Bay. > >> >> Add clk_ops for SD, EMMC and SDIO operations. > >> > > >> >The above commit message doesn't really tell why or what goes on here. > >> >Can please try to clarify that. > >> > >> We missed out the clock operation handling patch (Commit 16ada730a759) > >> and did not handle the clk_ops for Keem Bay SOC devices. > >> These 2 patches (Commit 16ada730a759) and (Commit 36c6aadaae86) are > >merged at around the same time. > >> We catch the issue later when trying to boot into v5.10-rc1 with Keem Bay > >EVM. > >> That is why I created this patch to handle the clk_ops for Keem Bay devices. > > > >Alright, so the previous changes were simply not sufficient to complete the > >support. > > > >> > >> > > >> >> > >> >> Fixes: 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for Intel > >> >> Keem > >> >> Bay") > >> > > >> >Is $subject patch fixing a bug/regression? > >> > >> This is to fix issue on previous commit. It is a bug fix. > > > >Has it ever worked? > > Yes it is working. Tested with Keem Bay EVM. > SD/EMMC/SDIO devices registered successfully with this fix. I am not asking if $subject patch makes it work, I understand that it does. > > > > >Did the commit you point to with the fixes tag, break some other existing > >functionality? > > It should not break other existing functionality. > This is specific for Keem Bay SOC only. So, you are saying that it has never worked for Keem Bay? But $subject patch makes it work? Can you please re-spin and update the commit message to better reflect what goes on, then I can apply. Kind regards Uffe
Hi Ulf, >-----Original Message----- >From: Ulf Hansson <ulf.hansson@linaro.org> >Sent: Wednesday, November 18, 2020 1:14 AM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; Raja >Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; Wan >Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box ><david.e.box@linux.intel.com> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops for Keem Bay >SOC > >On Tue, 17 Nov 2020 at 14:46, Zulkifli, Muhammad Husaini ><muhammad.husaini.zulkifli@intel.com> wrote: >> >> Hi Ulf, >> >> >-----Original Message----- >> >From: Ulf Hansson <ulf.hansson@linaro.org> >> >Sent: Tuesday, November 17, 2020 7:27 PM >> >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- >> >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >> >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; >> >Raja Subramanian, Lakshmi Bai >> ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad >> >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box >> ><david.e.box@linux.intel.com> >> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops >> >for Keem Bay SOC >> > >> >On Mon, 16 Nov 2020 at 15:22, Zulkifli, Muhammad Husaini >> ><muhammad.husaini.zulkifli@intel.com> wrote: >> >> >> >> Hi Ulf, >> >> >> >> Thanks for your review comments. I replied inline >> >> >> >> >-----Original Message----- >> >> >From: Ulf Hansson <ulf.hansson@linaro.org> >> >> >Sent: Monday, November 16, 2020 7:41 PM >> >> >To: Zulkifli, Muhammad Husaini >> >> ><muhammad.husaini.zulkifli@intel.com> >> >> >Cc: linux-mmc@vger.kernel.org; Linux ARM <linux-arm- >> >> >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >> >> >kernel@vger.kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; >> >> >Raja Subramanian, Lakshmi Bai >> >> ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan >Ahmad >> >> >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; David E. Box >> >> ><david.e.box@linux.intel.com> >> >> >Subject: Re: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops >> >> >for Keem Bay SOC >> >> > >> >> >On Fri, 6 Nov 2020 at 05:10, <muhammad.husaini.zulkifli@intel.com> >wrote: >> >> >> >> >> >> From: Muhammad Husaini Zulkifli >> >> >> <muhammad.husaini.zulkifli@intel.com> >> >> >> >> >> >> Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock >> >> >> operations >> >> >> handling") introduces platform specific SDHCI clock operation. >> >> >> >> >> >> This patch declares the clock operation for Keem Bay. >> >> >> Add clk_ops for SD, EMMC and SDIO operations. >> >> > >> >> >The above commit message doesn't really tell why or what goes on here. >> >> >Can please try to clarify that. >> >> >> >> We missed out the clock operation handling patch (Commit >> >> 16ada730a759) and did not handle the clk_ops for Keem Bay SOC devices. >> >> These 2 patches (Commit 16ada730a759) and (Commit 36c6aadaae86) >> >> are >> >merged at around the same time. >> >> We catch the issue later when trying to boot into v5.10-rc1 with >> >> Keem Bay >> >EVM. >> >> That is why I created this patch to handle the clk_ops for Keem Bay >devices. >> > >> >Alright, so the previous changes were simply not sufficient to >> >complete the support. >> > >> >> >> >> > >> >> >> >> >> >> Fixes: 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for >> >> >> Intel Keem >> >> >> Bay") >> >> > >> >> >Is $subject patch fixing a bug/regression? >> >> >> >> This is to fix issue on previous commit. It is a bug fix. >> > >> >Has it ever worked? >> >> Yes it is working. Tested with Keem Bay EVM. >> SD/EMMC/SDIO devices registered successfully with this fix. > >I am not asking if $subject patch makes it work, I understand that it does. > >> >> > >> >Did the commit you point to with the fixes tag, break some other >> >existing functionality? >> >> It should not break other existing functionality. >> This is specific for Keem Bay SOC only. > >So, you are saying that it has never worked for Keem Bay? But $subject patch >makes it work? It is working for Keem Bay before until the Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations handling") came into the picture. Both Commit 16ada730a759 ("mmc: sdhci-of-arasan: Modify clock operations handling") and Commit 36c6aadaae86 ("mmc: sdhci-of-arasan: Add support for Intel Keem") are merged about the same time. We did not have "Modify clock operations handling" patch when rebasing and testing during upstreaming process. I noticed of this issue when using latest mainline v5.10-rcx recently. > >Can you please re-spin and update the commit message to better reflect what >goes on, then I can apply. Noted. I will update the commit message and re-send again. Thanks > >Kind regards >Uffe
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index 829ccef87426..012d52e1abee 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -1199,16 +1199,19 @@ static struct sdhci_arasan_of_data sdhci_arasan_versal_data = { static struct sdhci_arasan_of_data intel_keembay_emmc_data = { .soc_ctl_map = &intel_keembay_soc_ctl_map, .pdata = &sdhci_keembay_emmc_pdata, + .clk_ops = &arasan_clk_ops, }; static struct sdhci_arasan_of_data intel_keembay_sd_data = { .soc_ctl_map = &intel_keembay_soc_ctl_map, .pdata = &sdhci_keembay_sd_pdata, + .clk_ops = &arasan_clk_ops, }; static struct sdhci_arasan_of_data intel_keembay_sdio_data = { .soc_ctl_map = &intel_keembay_soc_ctl_map, .pdata = &sdhci_keembay_sdio_pdata, + .clk_ops = &arasan_clk_ops, }; static const struct of_device_id sdhci_arasan_of_match[] = {