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

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

[Heiner Kallweit]

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);
 void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card);
 
 #endif /* LINUX_MMC_CORE_H */
-- 
2.11.0

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

[Heiner Kallweit]

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);
 void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card);
 
 #endif /* LINUX_MMC_CORE_H */
-- 
2.11.0

[PATCH 2/4] mmc: pwrseq: implement reset operation in pwrseq_emmc

In pwrseq_emmc a reset function exists already and just needs to
be exposed as reset hook.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/core/pwrseq_emmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
index adc9c0c6..a30e7352 100644
--- a/drivers/mmc/core/pwrseq_emmc.c
+++ b/drivers/mmc/core/pwrseq_emmc.c
@@ -57,6 +57,7 @@ static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
 
 static const struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
 	.post_power_on = mmc_pwrseq_emmc_reset,
+	.reset = mmc_pwrseq_emmc_reset,
 };
 
 static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
-- 
2.11.0

[PATCH 2/4] mmc: pwrseq: implement reset operation in pwrseq_emmc

[Heiner Kallweit]

In pwrseq_emmc a reset function exists already and just needs to
be exposed as reset hook.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/core/pwrseq_emmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
index adc9c0c6..a30e7352 100644
--- a/drivers/mmc/core/pwrseq_emmc.c
+++ b/drivers/mmc/core/pwrseq_emmc.c
@@ -57,6 +57,7 @@ static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
 
 static const struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
 	.post_power_on = mmc_pwrseq_emmc_reset,
+	.reset = mmc_pwrseq_emmc_reset,
 };
 
 static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
-- 
2.11.0

[PATCH 3/4] mmc: meson-gx: add pwrseq-based hw_reset

[Heiner Kallweit]

Use new function mmc_hw_reset_pwrseq as hw_reset operation.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5eca88bc..260a8ea8 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -716,6 +716,7 @@ static const struct mmc_host_ops meson_mmc_ops = {
 	.request	= meson_mmc_request,
 	.set_ios	= meson_mmc_set_ios,
 	.get_cd         = meson_mmc_get_cd,
+	.hw_reset	= mmc_hw_reset_pwrseq,
 };
 
 static int meson_mmc_probe(struct platform_device *pdev)
-- 
2.11.0

[PATCH 3/4] mmc: meson-gx: add pwrseq-based hw_reset

[Heiner Kallweit]

Use new function mmc_hw_reset_pwrseq as hw_reset operation.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5eca88bc..260a8ea8 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -716,6 +716,7 @@ static const struct mmc_host_ops meson_mmc_ops = {
 	.request	= meson_mmc_request,
 	.set_ios	= meson_mmc_set_ios,
 	.get_cd         = meson_mmc_get_cd,
+	.hw_reset	= mmc_hw_reset_pwrseq,
 };
 
 static int meson_mmc_probe(struct platform_device *pdev)
-- 
2.11.0

[PATCH 4/4] ARM64: dts: meson-gxbb-odroidc2: add hw-reset capability flag for emmc port

[Heiner Kallweit]

Add hw-reset capability flag for emmc port.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index c59403ad..2b68994a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -219,6 +219,7 @@
 	pinctrl-names = "default";
 
 	bus-width = <8>;
+	cap-mmc-hw-reset;
 	cap-sd-highspeed;
 	max-frequency = <200000000>;
 	non-removable;
-- 
2.11.0

[PATCH 4/4] ARM64: dts: meson-gxbb-odroidc2: add hw-reset capability flag for emmc port

[Heiner Kallweit]

Add hw-reset capability flag for emmc port.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index c59403ad..2b68994a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -219,6 +219,7 @@
 	pinctrl-names = "default";
 
 	bus-width = <8>;
+	cap-mmc-hw-reset;
 	cap-sd-highspeed;
 	max-frequency = <200000000>;
 	non-removable;
-- 
2.11.0

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

[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?

>  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

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

[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?

>  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

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

[Heiner Kallweit]

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
> 

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

[Heiner Kallweit]

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
> 

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

[Heiner Kallweit]

Am 15.03.2017 um 22:42 schrieb Heiner Kallweit:
> 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.
> 
Other alternative:

typedef void (*hw_reset_t)(struct mmc_host *host);

hw_reset_t mmc_get_hw_reset(struct mmc_host *host)
{
       struct mmc_pwrseq *p = host->pwrseq;

       if (host->ops->hw_reset)
               return host->ops->hw_reset;
       else if (p && p->ops->reset)
               return p->ops->reset;
       else
               return NULL;
}

And then use what mmc_get_hw_reset returns instead of using
host->ops->hw_reset directly.

How do you like this one?

>>>  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
>>
> 

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

[Heiner Kallweit]

Am 15.03.2017 um 22:42 schrieb Heiner Kallweit:
> 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.
> 
Other alternative:

typedef void (*hw_reset_t)(struct mmc_host *host);

hw_reset_t mmc_get_hw_reset(struct mmc_host *host)
{
       struct mmc_pwrseq *p = host->pwrseq;

       if (host->ops->hw_reset)
               return host->ops->hw_reset;
       else if (p && p->ops->reset)
               return p->ops->reset;
       else
               return NULL;
}

And then use what mmc_get_hw_reset returns instead of using
host->ops->hw_reset directly.

How do you like this one?

>>>  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
>>
> 

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

[Ulf Hansson]

[...]

>>> @@ -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.

No thanks. :-)

[...]

Kind regards
Uffe

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

[Ulf Hansson]

[...]

>>> @@ -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.

No thanks. :-)

[...]

Kind regards
Uffe

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

[Ulf Hansson]

[...]

>>
> Other alternative:
>
> typedef void (*hw_reset_t)(struct mmc_host *host);
>
> hw_reset_t mmc_get_hw_reset(struct mmc_host *host)
> {
>        struct mmc_pwrseq *p = host->pwrseq;
>
>        if (host->ops->hw_reset)
>                return host->ops->hw_reset;
>        else if (p && p->ops->reset)
>                return p->ops->reset;
>        else
>                return NULL;
> }
>
> And then use what mmc_get_hw_reset returns instead of using
> host->ops->hw_reset directly.
>
> How do you like this one?

Well, I would just rather do these checks and call the correct
callback in mmc_reset(). There is no need to invent a function
returning a function pointer when we call it directly instead.

Additionally, I wonder whether it could make sense to allow both
callbacks to co-exist and then call both. This would give the mmc host
some options for doing some internal reset as well, while the pwrseq
could handle the actual GPIO pin.

What do you think?

Kind regards
Uffe

>
>>>>  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
>>>
>>
>

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

[Ulf Hansson]

[...]

>>
> Other alternative:
>
> typedef void (*hw_reset_t)(struct mmc_host *host);
>
> hw_reset_t mmc_get_hw_reset(struct mmc_host *host)
> {
>        struct mmc_pwrseq *p = host->pwrseq;
>
>        if (host->ops->hw_reset)
>                return host->ops->hw_reset;
>        else if (p && p->ops->reset)
>                return p->ops->reset;
>        else
>                return NULL;
> }
>
> And then use what mmc_get_hw_reset returns instead of using
> host->ops->hw_reset directly.
>
> How do you like this one?

Well, I would just rather do these checks and call the correct
callback in mmc_reset(). There is no need to invent a function
returning a function pointer when we call it directly instead.

Additionally, I wonder whether it could make sense to allow both
callbacks to co-exist and then call both. This would give the mmc host
some options for doing some internal reset as well, while the pwrseq
could handle the actual GPIO pin.

What do you think?

Kind regards
Uffe

>
>>>>  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
>>>
>>
>