RE: [PATCH v1 1/1] mmc: sdhci-of-arasan: Specify .clk_ops for Keem Bay SOC

[Zulkifli, Muhammad Husaini]

Hi Ulf,

>-----Original Message-----
>From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>Sent: Wednesday, November 18, 2020 1:14 AM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@xxxxxxxxx>
>Cc: linux-mmc@xxxxxxxxxxxxxxx; Linux ARM <linux-arm-
>kernel@xxxxxxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
>kernel@xxxxxxxxxxxxxxx>; Hunter, Adrian <adrian.hunter@xxxxxxxxx>; Raja
>Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@xxxxxxxxx>; Wan
>Mohamad, Wan Ahmad Zainie
><wan.ahmad.zainie.wan.mohamad@xxxxxxxxx>; David E. Box
><david.e.box@xxxxxxxxxxxxxxx>
>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@xxxxxxxxx> wrote:
>>
>> Hi Ulf,
>>
>> >-----Original Message-----
>> >From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>> >Sent: Tuesday, November 17, 2020 7:27 PM
>> >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@xxxxxxxxx>
>> >Cc: linux-mmc@xxxxxxxxxxxxxxx; Linux ARM <linux-arm-
>> >kernel@xxxxxxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
>> >kernel@xxxxxxxxxxxxxxx>; Hunter, Adrian <adrian.hunter@xxxxxxxxx>;
>> >Raja Subramanian, Lakshmi Bai
>> ><lakshmi.bai.raja.subramanian@xxxxxxxxx>; Wan Mohamad, Wan Ahmad
>> >Zainie <wan.ahmad.zainie.wan.mohamad@xxxxxxxxx>; David E. Box
>> ><david.e.box@xxxxxxxxxxxxxxx>
>> >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@xxxxxxxxx> wrote:
>> >>
>> >> Hi Ulf,
>> >>
>> >> Thanks for your review comments. I replied inline
>> >>
>> >> >-----Original Message-----
>> >> >From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>> >> >Sent: Monday, November 16, 2020 7:41 PM
>> >> >To: Zulkifli, Muhammad Husaini
>> >> ><muhammad.husaini.zulkifli@xxxxxxxxx>
>> >> >Cc: linux-mmc@xxxxxxxxxxxxxxx; Linux ARM <linux-arm-
>> >> >kernel@xxxxxxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
>> >> >kernel@xxxxxxxxxxxxxxx>; Hunter, Adrian <adrian.hunter@xxxxxxxxx>;
>> >> >Raja Subramanian, Lakshmi Bai
>> >> ><lakshmi.bai.raja.subramanian@xxxxxxxxx>; Wan Mohamad, Wan
>Ahmad
>> >> >Zainie <wan.ahmad.zainie.wan.mohamad@xxxxxxxxx>; David E. Box
>> >> ><david.e.box@xxxxxxxxxxxxxxx>
>> >> >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@xxxxxxxxx>
>wrote:
>> >> >>
>> >> >> From: Muhammad Husaini Zulkifli
>> >> >> <muhammad.husaini.zulkifli@xxxxxxxxx>
>> >> >>
>> >> >> 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