Hi Ulf
Thanks for your review comments.
On 10/16/2017 1:56 PM, Ulf Hansson wrote:
> On 12 October 2017 at 12:32, Shah, Nehal-bakulchandra
> <Nehal-Bakulchandra.shah@xxxxxxx> wrote:
>> From: Shah Nehal-Bakulchandra <Nehal-bakulchandra.Shah@xxxxxxx>
>>
>> This patch supports HS400 for AMD upcoming emmc 5.0 controller.The
>> HS400 and HS200 mode requires hardware work around also. This patch
>> adds the quirks for the same.
>
> By looking at the changes in the patch, obviously you make changes to
> the core, but I think the reasons to why explicitly needs to be stated
> also in the change log.
I will update the change log with detailed description.
>>
>> Signed-off-by: Shah Nehal-Bakulchandra <Nehal-bakulchandra.Shah@xxxxxxx>
>> ---
>> drivers/mmc/core/mmc.c | 6 +--
>> drivers/mmc/host/sdhci-acpi.c | 89 +++++++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/host/sdhci.c | 3 +-
>> drivers/mmc/host/sdhci.h | 5 +++
>> include/linux/mmc/host.h | 6 +++
>> 5 files changed, 105 insertions(+), 4 deletions(-)
>
> Please split the core changes into a separate patch.
Sure i will split the core changes into a separate patch.
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 4ffea14..7bf3736 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1164,7 +1164,7 @@ static int mmc_select_hs400(struct mmc_card *card)
>>
>> /* Set host controller to HS timing */
>> mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -
>> + host->ios.transition = HS200_TO_HS_TO_HS400;
>> /* Reduce frequency to HS frequency */
>> max_dtr = card->ext_csd.hs_max_dtr;
>> mmc_set_clock(host, max_dtr);
>> @@ -1196,7 +1196,7 @@ static int mmc_select_hs400(struct mmc_card *card)
>> mmc_hostname(host), err);
>> return err;
>> }
>> -
>> + host->ios.transition = SWITCHING_TO_HS400;
>> /* Set host controller to HS400 timing and frequency */
>> mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>> mmc_set_bus_speed(card);
>> @@ -1204,7 +1204,7 @@ static int mmc_select_hs400(struct mmc_card *card)
>> err = mmc_switch_status(card);
>> if (err)
>> goto out_err;
>> -
>> + host->ios.transition = SWITCHED_TO_HS400;
>> return 0;
>
> We have some other host drivers that also needs to follow a specific
> sequence while switching to HS400. Those drivers makes use of the
> ->prepare_hs400_tuning() callback. Have you explored that option, and
> if so, why exactly doesn't it work for you?
>
> [...]
>
I am aware about prepare_hs400_tuning(), however in AMD Platform case, some settings are required after it switches to HS400. HS400 switching is also
required intermediate transition, so in past i discussed with Adrian ( very initial patch ) and he suggested to use ios with transition.
>> /*
>> * End of controller registers.
>> */
>> @@ -571,6 +574,8 @@ struct sdhci_ops {
>> void (*set_bus_width)(struct sdhci_host *host, int width);
>> void (*platform_send_init_74_clocks)(struct sdhci_host *host,
>> u8 power_mode);
>> + /* ios for transiton phase for going to hs400 */
>> + void (*set_platform_hs400_transition)(struct sdhci_host *host);
>
> In general we avoid adding new sdhci callbacks, but instead we try to
> use the sdhci core as a set of library functions.
>
> However, I leave this to managed by Adrian, as we decide this on case
> by case basis.
>
I will wait for Adrian's comments also. Based on that will submit the new patch.
>> unsigned int (*get_ro)(struct sdhci_host *host);
>> void (*reset)(struct sdhci_host *host, u8 mask);
>> int (*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index ebd1ceb..0d0d5d3 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -77,6 +77,12 @@ struct mmc_ios {
>> #define MMC_SET_DRIVER_TYPE_D 3
>>
>> bool enhanced_strobe; /* hs400es selection */
>> +
>> + unsigned int transition; /* track transition modes (hs200 hs400) */
>> +
>> +#define HS200_TO_HS_TO_HS400 1
>> +#define SWITCHING_TO_HS400 2
>> +#define SWITCHED_TO_HS400 3
>> };
>>
>> struct mmc_host;
>> --
>> 1.9.1
>>
>
> Kind regards
> Uffe
>
Regards
Nehal Shah
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
