Re: [PATCH 1/4] mmc: pwrseq: add op reset to struct mmc_pwrseq_ops

[Heiner Kallweit <hkallweit1@gmail.com>]

Am 15.03.2017 um 11:50 schrieb Ulf Hansson:
> On 8 February 2017 at 22:48, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Add an operation reset to struct mmc_pwrseq_ops and related wrappers.
>>
>> Don't expose the pwrseq-internal reset function directly to kernel
>> users but just to the mmc core.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/core/core.c   | 6 ++++++
>>  drivers/mmc/core/pwrseq.c | 8 ++++++++
>>  drivers/mmc/core/pwrseq.h | 3 +++
>>  include/linux/mmc/core.h  | 1 +
>>  4 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 926e0fde..5d5fe591 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1869,6 +1869,12 @@ void mmc_power_cycle(struct mmc_host *host, u32 ocr)
>>         mmc_power_up(host, ocr);
>>  }
>>
>> +void mmc_hw_reset_pwrseq(struct mmc_host *host)
>> +{
>> +       mmc_pwrseq_reset(host);
>> +}
>> +EXPORT_SYMBOL(mmc_hw_reset_pwrseq);
>> +
>>  /*
>>   * Cleanup when the last reference to the bus operator is dropped.
>>   */
>> diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
>> index 9386c477..e3ad30fa 100644
>> --- a/drivers/mmc/core/pwrseq.c
>> +++ b/drivers/mmc/core/pwrseq.c
>> @@ -76,6 +76,14 @@ void mmc_pwrseq_power_off(struct mmc_host *host)
>>                 pwrseq->ops->power_off(host);
>>  }
>>
>> +void mmc_pwrseq_reset(struct mmc_host *host)
>> +{
>> +       struct mmc_pwrseq *pwrseq = host->pwrseq;
>> +
>> +       if (pwrseq && pwrseq->ops->reset)
>> +               pwrseq->ops->reset(host);
>> +}
>> +
>>  void mmc_pwrseq_free(struct mmc_host *host)
>>  {
>>         struct mmc_pwrseq *pwrseq = host->pwrseq;
>> diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
>> index 39c911aa..819386f4 100644
>> --- a/drivers/mmc/core/pwrseq.h
>> +++ b/drivers/mmc/core/pwrseq.h
>> @@ -18,6 +18,7 @@ struct mmc_pwrseq_ops {
>>         void (*pre_power_on)(struct mmc_host *host);
>>         void (*post_power_on)(struct mmc_host *host);
>>         void (*power_off)(struct mmc_host *host);
>> +       void (*reset)(struct mmc_host *host);
>>  };
>>
>>  struct mmc_pwrseq {
>> @@ -36,6 +37,7 @@ int mmc_pwrseq_alloc(struct mmc_host *host);
>>  void mmc_pwrseq_pre_power_on(struct mmc_host *host);
>>  void mmc_pwrseq_post_power_on(struct mmc_host *host);
>>  void mmc_pwrseq_power_off(struct mmc_host *host);
>> +void mmc_pwrseq_reset(struct mmc_host *host);
>>  void mmc_pwrseq_free(struct mmc_host *host);
>>
>>  #else
>> @@ -49,6 +51,7 @@ static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
>>  static inline void mmc_pwrseq_pre_power_on(struct mmc_host *host) {}
>>  static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
>>  static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
>> +static inline void mmc_pwrseq_reset(struct mmc_host *host) {}
>>  static inline void mmc_pwrseq_free(struct mmc_host *host) {}
>>
>>  #endif
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index a0c63ea2..8dbbd7a1 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -166,6 +166,7 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd,
>>                 int retries);
>>
>>  int mmc_hw_reset(struct mmc_host *host);
>> +void mmc_hw_reset_pwrseq(struct mmc_host *host);
> 
> Don't you think we can make this transparent to mmc host drivers,
> instead of them having to assign their host_ops->hw_reset() callback
> to this new API? Because I guess that's the though!?
> 
> In principle the mmc core already have all the information it needs,
> as to understand when the eMMC pwrseq should be invoked. Or perhaps
> the code may become a bit too messy for that?
> 
The easiest way I could think of:
If host->ops->hw_reset is NULL overwrite it with pwrseq->ops->reset
in mmc_add_host. But host->ops is defined es being const, so the compiler
won't allow us to do this. Of course we could do some casting but usually
I try to avoid hacks like casting away const qualifiers.

>>  void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card);
>>
>>  #endif /* LINUX_MMC_CORE_H */
>> --
>> 2.11.0
>>
>>
> 
> Kind regards
> Uffe
>