Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
On Wed, 25 Sep 2019 at 13:27, wrote: > > On 2019-09-12 22:15, Bjorn Andersson wrote: > > On Fri 06 Sep 05:47 PDT 2019, Pradeep P V K wrote: > > > >> Vote for the MSM bus bandwidth required by SDHC driver > >> based on the clock frequency and bus width of the card. > >> Otherwise,the system clocks may run at minimum clock speed > >> and thus affecting the performance. > >> > >> This change is based on Georgi Djakov [RFC] > >> (https://lkml.org/lkml/2018/10/11/499) > >> > >> Signed-off-by: Sahitya Tummala > >> Signed-off-by: Subhash Jadavani > >> Signed-off-by: Veerabhadrarao Badiganti > >> Signed-off-by: Pradeep P V K > > > > This says that Sahitya wrote the patch, then Subhash handled it, then > > Veerabhadrarao handled it and finally you handled it; but you're at the > > same time listed as author (by From:). > > > > Please see section 12 on Co-Developed-by in submitting-patches.rst > > > Thanks Bjorn, i will update this on my next patch set. > >> --- > >> drivers/mmc/host/sdhci-msm.c | 393 > >> ++- > > > > This patch implements support for requesting bandwidth to the register > > space and for the controllers access to DDR. To me this seems like > > common requirements for any mmc controller, can this functionality be > > provided by the mmc/sdhci common code? > > > > Regards, > > Bjorn > > > Yes, this can be provided in common code but the bandwidth calculations > (arbitrated value or average bandwidth and instantaneous value or peak > bandwidth) for bus vote will > consider various parameters like voltage corners, clock domains, clock > plans etc. which may differ from > vendor to vendor and target to target. So, these values should be > updated properly and correctly (considering all the parameters) > if it brings to common area. > > Hence the reason for implementing this in sdhci-msm.c file. > It would be really helpful if you could suggest your insights on where > and how exactly this needs to be > implemented in common code area. > > Hi Ulf and Adrian, > > Can you please also suggest your recommendations on this ? I am not sure where to put it at this point. Perhaps we can just start with making it specific for sdhci-msm, then when new users come into play, we can consider moving it to a common place. [...] Kind regards Uffe
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
On 2019-09-12 22:15, Bjorn Andersson wrote:
On Fri 06 Sep 05:47 PDT 2019, Pradeep P V K wrote:
Vote for the MSM bus bandwidth required by SDHC driver
based on the clock frequency and bus width of the card.
Otherwise,the system clocks may run at minimum clock speed
and thus affecting the performance.
This change is based on Georgi Djakov [RFC]
(https://lkml.org/lkml/2018/10/11/499)
Signed-off-by: Sahitya Tummala
Signed-off-by: Subhash Jadavani
Signed-off-by: Veerabhadrarao Badiganti
Signed-off-by: Pradeep P V K
This says that Sahitya wrote the patch, then Subhash handled it, then
Veerabhadrarao handled it and finally you handled it; but you're at the
same time listed as author (by From:).
Please see section 12 on Co-Developed-by in submitting-patches.rst
Thanks Bjorn, i will update this on my next patch set.
---
drivers/mmc/host/sdhci-msm.c | 393
++-
This patch implements support for requesting bandwidth to the register
space and for the controllers access to DDR. To me this seems like
common requirements for any mmc controller, can this functionality be
provided by the mmc/sdhci common code?
Regards,
Bjorn
Yes, this can be provided in common code but the bandwidth calculations
(arbitrated value or average bandwidth and instantaneous value or peak
bandwidth) for bus vote will
consider various parameters like voltage corners, clock domains, clock
plans etc. which may differ from
vendor to vendor and target to target. So, these values should be
updated properly and correctly (considering all the parameters)
if it brings to common area.
Hence the reason for implementing this in sdhci-msm.c file.
It would be really helpful if you could suggest your insights on where
and how exactly this needs to be
implemented in common code area.
Hi Ulf and Adrian,
Can you please also suggest your recommendations on this ?
Thanks and Regards,
Pradeep
1 file changed, 390 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-msm.c
b/drivers/mmc/host/sdhci-msm.c
index b75c82d..71515ca 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -122,6 +123,9 @@
#define msm_host_writel(msm_host, val, host, offset) \
msm_host->var_ops->msm_writel_relaxed(val, host, offset)
+#define SDHC_DDR "sdhc-ddr"
+#define CPU_SDHC "cpu-sdhc"
+
struct sdhci_msm_offset {
u32 core_hc_mode;
u32 core_mci_data_cnt;
@@ -228,6 +232,31 @@ struct sdhci_msm_variant_info {
const struct sdhci_msm_offset *offset;
};
+struct msm_bus_vectors {
+ uint64_t ab;
+ uint64_t ib;
+};
+
+struct msm_bus_path {
+ unsigned int num_paths;
+ struct msm_bus_vectors *vec;
+};
+
+struct sdhci_msm_bus_vote_data {
+ const char *name;
+ unsigned int num_usecase;
+ struct msm_bus_path *usecase;
+
+ unsigned int *bw_vecs;
+ unsigned int bw_vecs_size;
+
+ struct icc_path *sdhc_ddr;
+ struct icc_path *cpu_sdhc;
+
+ uint32_t curr_vote;
+
+};
+
struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -253,8 +282,13 @@ struct sdhci_msm_host {
const struct sdhci_msm_offset *offset;
bool use_cdr;
u32 transfer_mode;
+ bool skip_bus_bw_voting;
+ struct sdhci_msm_bus_vote_data *bus_vote_data;
+ struct delayed_work bus_vote_work;
};
+static void sdhci_msm_bus_voting(struct sdhci_host *host, u32
enable);
+
static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct
sdhci_host *host)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1557,6 +1591,8 @@ static void sdhci_msm_set_clock(struct
sdhci_host *host, unsigned int clock)
msm_set_clock_rate_for_bus_mode(host, clock);
out:
+ if (!msm_host->skip_bus_bw_voting)
+ sdhci_msm_bus_voting(host, !!clock);
__sdhci_msm_set_clock(host, clock);
}
@@ -1678,6 +1714,341 @@ static void
sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
}
+static int sdhci_msm_dt_get_array(struct device *dev, const char
*prop_name,
+u32 **bw_vecs, int *len, u32 size)
+{
+ int ret = 0;
+ struct device_node *np = dev->of_node;
+ size_t sz;
+ u32 *arr = NULL;
+
+ if (!of_get_property(np, prop_name, len)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ sz = *len = *len / sizeof(*arr);
+ if (sz <= 0 || (size > 0 && (sz > size))) {
+ dev_err(dev, "%s invalid size\n", prop_name);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ arr = devm_kzalloc(dev, sz * sizeof(*arr), GFP_KERNEL);
+ if (!arr) {
+ ret = -ENOMEM;
+ goto out;
+ }
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
On 2019-09-12 18:26, Georgi Djakov wrote: Hi Pradeep, Thanks for the patch! On 9/6/19 15:47, Pradeep P V K wrote: Vote for the MSM bus bandwidth required by SDHC driver based on the clock frequency and bus width of the card. Otherwise,the system clocks may run at minimum clock speed and thus affecting the performance. This change is based on Georgi Djakov [RFC] (https://lkml.org/lkml/2018/10/11/499) I am just wondering whether do we really need to predefine the bandwidth values in DT? Can't we use the computations from the above patch or is there any problem with that approach? Thanks, Georgi Hi Georgi, By using the direct required bandwidth(bw / 1000) values, it will not guarantee that all the NOC clocks are running in the same voltage corner as required, which is very crucial for power concern devices like Mobiles etc. Also, it will not guarantee that the value passed is in proper Clock Plans domain there by effecting the requested Bandwidth. I think, you already aware of these consequences on using direct bandwidth values for RPMh based devices. The value the we passed in DT will make sure that all the NOC clocks between the end points are running in the same voltage corners as required and also it will guarantee that the requested BW's for the clients are obtained. Hence the reason for passing the predefined bandwidth values in DT. Thanks and Regards, Pradeep
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
On Fri 06 Sep 05:47 PDT 2019, Pradeep P V K wrote:
> Vote for the MSM bus bandwidth required by SDHC driver
> based on the clock frequency and bus width of the card.
> Otherwise,the system clocks may run at minimum clock speed
> and thus affecting the performance.
>
> This change is based on Georgi Djakov [RFC]
> (https://lkml.org/lkml/2018/10/11/499)
>
> Signed-off-by: Sahitya Tummala
> Signed-off-by: Subhash Jadavani
> Signed-off-by: Veerabhadrarao Badiganti
> Signed-off-by: Pradeep P V K
This says that Sahitya wrote the patch, then Subhash handled it, then
Veerabhadrarao handled it and finally you handled it; but you're at the
same time listed as author (by From:).
Please see section 12 on Co-Developed-by in submitting-patches.rst
> ---
> drivers/mmc/host/sdhci-msm.c | 393
> ++-
This patch implements support for requesting bandwidth to the register
space and for the controllers access to DDR. To me this seems like
common requirements for any mmc controller, can this functionality be
provided by the mmc/sdhci common code?
Regards,
Bjorn
> 1 file changed, 390 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index b75c82d..71515ca 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -122,6 +123,9 @@
> #define msm_host_writel(msm_host, val, host, offset) \
> msm_host->var_ops->msm_writel_relaxed(val, host, offset)
>
> +#define SDHC_DDR "sdhc-ddr"
> +#define CPU_SDHC "cpu-sdhc"
> +
> struct sdhci_msm_offset {
> u32 core_hc_mode;
> u32 core_mci_data_cnt;
> @@ -228,6 +232,31 @@ struct sdhci_msm_variant_info {
> const struct sdhci_msm_offset *offset;
> };
>
> +struct msm_bus_vectors {
> + uint64_t ab;
> + uint64_t ib;
> +};
> +
> +struct msm_bus_path {
> + unsigned int num_paths;
> + struct msm_bus_vectors *vec;
> +};
> +
> +struct sdhci_msm_bus_vote_data {
> + const char *name;
> + unsigned int num_usecase;
> + struct msm_bus_path *usecase;
> +
> + unsigned int *bw_vecs;
> + unsigned int bw_vecs_size;
> +
> + struct icc_path *sdhc_ddr;
> + struct icc_path *cpu_sdhc;
> +
> + uint32_t curr_vote;
> +
> +};
> +
> struct sdhci_msm_host {
> struct platform_device *pdev;
> void __iomem *core_mem; /* MSM SDCC mapped address */
> @@ -253,8 +282,13 @@ struct sdhci_msm_host {
> const struct sdhci_msm_offset *offset;
> bool use_cdr;
> u32 transfer_mode;
> + bool skip_bus_bw_voting;
> + struct sdhci_msm_bus_vote_data *bus_vote_data;
> + struct delayed_work bus_vote_work;
> };
>
> +static void sdhci_msm_bus_voting(struct sdhci_host *host, u32 enable);
> +
> static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct
> sdhci_host *host)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1557,6 +1591,8 @@ static void sdhci_msm_set_clock(struct sdhci_host
> *host, unsigned int clock)
>
> msm_set_clock_rate_for_bus_mode(host, clock);
> out:
> + if (!msm_host->skip_bus_bw_voting)
> + sdhci_msm_bus_voting(host, !!clock);
> __sdhci_msm_set_clock(host, clock);
> }
>
> @@ -1678,6 +1714,341 @@ static void sdhci_msm_set_regulator_caps(struct
> sdhci_msm_host *msm_host)
> pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
> }
>
> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
> + u32 **bw_vecs, int *len, u32 size)
> +{
> + int ret = 0;
> + struct device_node *np = dev->of_node;
> + size_t sz;
> + u32 *arr = NULL;
> +
> + if (!of_get_property(np, prop_name, len)) {
> + ret = -EINVAL;
> + goto out;
> + }
> + sz = *len = *len / sizeof(*arr);
> + if (sz <= 0 || (size > 0 && (sz > size))) {
> + dev_err(dev, "%s invalid size\n", prop_name);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + arr = devm_kzalloc(dev, sz * sizeof(*arr), GFP_KERNEL);
> + if (!arr) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + ret = of_property_read_u32_array(np, prop_name, arr, sz);
> + if (ret < 0) {
> + dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
> + goto out;
> + }
> + *bw_vecs = arr;
> +out:
> + if (ret)
> + *len = 0;
> + return ret;
> +}
> +
> +/* Returns required bandwidth in Bytes per Sec */
> +static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
> + struct mmc_ios *ios)
> +{
> + unsigned long bw;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + bw = msm_host->clk_rate;
> +
> +
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
On 2019-09-07 02:32, Stephen Boyd wrote:
Quoting p...@codeaurora.org (2019-09-06 05:51:54)
+Georgi Djakov
On 2019-09-06 18:17, Pradeep P V K wrote:
> diff --git a/drivers/mmc/host/sdhci-msm.c
> b/drivers/mmc/host/sdhci-msm.c
> index b75c82d..71515ca 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> #define msm_host_writel(msm_host, val, host, offset) \
> msm_host->var_ops->msm_writel_relaxed(val, host, offset)
>
> +#define SDHC_DDR "sdhc-ddr"
> +#define CPU_SDHC "cpu-sdhc"
Do you really need these defines? They're not used more than once or
twice and just seem to make the code harder to read.
Agree and will address this in my next patch set.
> +
> struct sdhci_msm_offset {
> u32 core_hc_mode;
> u32 core_mci_data_cnt;
> @@ -228,6 +232,31 @@ struct sdhci_msm_variant_info {
> const struct sdhci_msm_offset *offset;
> };
>
> +struct msm_bus_vectors {
> + uint64_t ab;
> + uint64_t ib;
> +};
> +
> +struct msm_bus_path {
> + unsigned int num_paths;
> + struct msm_bus_vectors *vec;
> +};
> +
> +struct sdhci_msm_bus_vote_data {
> + const char *name;
> + unsigned int num_usecase;
> + struct msm_bus_path *usecase;
> +
> + unsigned int *bw_vecs;
> + unsigned int bw_vecs_size;
> +
> + struct icc_path *sdhc_ddr;
> + struct icc_path *cpu_sdhc;
> +
> + uint32_t curr_vote;
> +
Please use u32 instead of uint32_t. Same comment for u64.
Ok. I will address this in my next patch set.
> +};
> +
> struct sdhci_msm_host {
> struct platform_device *pdev;
> void __iomem *core_mem; /* MSM SDCC mapped address */
> @@ -1678,6 +1714,341 @@ static void
> sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
> pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
> }
>
> +static int sdhci_msm_dt_get_array(struct device *dev, const char
> *prop_name,
> + u32 **bw_vecs, int *len, u32 size)
> +{
> + int ret = 0;
> + struct device_node *np = dev->of_node;
> + size_t sz;
> + u32 *arr = NULL;
> +
> + if (!of_get_property(np, prop_name, len)) {
> + ret = -EINVAL;
> + goto out;
> + }
> + sz = *len = *len / sizeof(*arr);
> + if (sz <= 0 || (size > 0 && (sz > size))) {
> + dev_err(dev, "%s invalid size\n", prop_name);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + arr = devm_kzalloc(dev, sz * sizeof(*arr), GFP_KERNEL);
> + if (!arr) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + ret = of_property_read_u32_array(np, prop_name, arr, sz);
> + if (ret < 0) {
> + dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
> + goto out;
> + }
> + *bw_vecs = arr;
> +out:
> + if (ret)
> + *len = 0;
> + return ret;
> +}
> +
> +/* Returns required bandwidth in Bytes per Sec */
> +static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
> + struct mmc_ios *ios)
> +{
> + unsigned long bw;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + bw = msm_host->clk_rate;
> +
> + if (ios->bus_width == MMC_BUS_WIDTH_4)
> + bw /= 2;
> + else if (ios->bus_width == MMC_BUS_WIDTH_1)
> + bw /= 8;
> +
> + return bw;
> +}
> +
> +static int sdhci_msm_bus_get_vote_for_bw(struct sdhci_msm_host *host,
> +unsigned int bw)
> +{
> + struct sdhci_msm_bus_vote_data *bvd = host->bus_vote_data;
> +
> + unsigned int *table = bvd->bw_vecs;
Should probably be a const bw_vecs pointer so that it can't be modified
after the fact.
Agree and will address this in my next patch set.
> + unsigned int size = bvd->bw_vecs_size;
> + int i;
> +
> + for (i = 0; i < size; i++) {
> + if (bw <= table[i])
> + break;
return i;
Ok. I will address this in my next patch set.
> + }
> +
return i - 1;
Ok. I will address this in my next patch set.
> + if (i && (i == size))
> + i--;
> +
> + return i;
And then this is useless.
Agree and will address this in my next patch set.
> +}
> +
> +/*
> + * This function must be called with host lock acquired.
> + * Caller of this function should also ensure that msm bus client
> + * handle is not null.
If it was NULL it would be pretty sad.
Agree but this is handled in the caller of this fun.
and will update in my next patch set.
> + */
> +static inline int sdhci_msm_bus_set_vote(struct sdhci_msm_host
> *msm_host,
> + int vote,
> + unsigned long *flags)
> +{
> + struct sdhci_host *host = platform_get_drvdata(msm_host->pdev);
> + struct sdhci_msm_bus_vote_data *bvd =
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
Hi Pradeep, Thanks for the patch! On 9/6/19 15:47, Pradeep P V K wrote: > Vote for the MSM bus bandwidth required by SDHC driver > based on the clock frequency and bus width of the card. > Otherwise,the system clocks may run at minimum clock speed > and thus affecting the performance. > > This change is based on Georgi Djakov [RFC] > (https://lkml.org/lkml/2018/10/11/499) I am just wondering whether do we really need to predefine the bandwidth values in DT? Can't we use the computations from the above patch or is there any problem with that approach? Thanks, Georgi
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
Quoting p...@codeaurora.org (2019-09-06 05:51:54)
> +Georgi Djakov
>
> On 2019-09-06 18:17, Pradeep P V K wrote:
> > diff --git a/drivers/mmc/host/sdhci-msm.c
> > b/drivers/mmc/host/sdhci-msm.c
> > index b75c82d..71515ca 100644
> > --- a/drivers/mmc/host/sdhci-msm.c
> > +++ b/drivers/mmc/host/sdhci-msm.c
> > #define msm_host_writel(msm_host, val, host, offset) \
> > msm_host->var_ops->msm_writel_relaxed(val, host, offset)
> >
> > +#define SDHC_DDR "sdhc-ddr"
> > +#define CPU_SDHC "cpu-sdhc"
Do you really need these defines? They're not used more than once or
twice and just seem to make the code harder to read.
> > +
> > struct sdhci_msm_offset {
> > u32 core_hc_mode;
> > u32 core_mci_data_cnt;
> > @@ -228,6 +232,31 @@ struct sdhci_msm_variant_info {
> > const struct sdhci_msm_offset *offset;
> > };
> >
> > +struct msm_bus_vectors {
> > + uint64_t ab;
> > + uint64_t ib;
> > +};
> > +
> > +struct msm_bus_path {
> > + unsigned int num_paths;
> > + struct msm_bus_vectors *vec;
> > +};
> > +
> > +struct sdhci_msm_bus_vote_data {
> > + const char *name;
> > + unsigned int num_usecase;
> > + struct msm_bus_path *usecase;
> > +
> > + unsigned int *bw_vecs;
> > + unsigned int bw_vecs_size;
> > +
> > + struct icc_path *sdhc_ddr;
> > + struct icc_path *cpu_sdhc;
> > +
> > + uint32_t curr_vote;
> > +
Please use u32 instead of uint32_t. Same comment for u64.
> > +};
> > +
> > struct sdhci_msm_host {
> > struct platform_device *pdev;
> > void __iomem *core_mem; /* MSM SDCC mapped address */
> > @@ -1678,6 +1714,341 @@ static void
> > sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
> > pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
> > }
> >
> > +static int sdhci_msm_dt_get_array(struct device *dev, const char
> > *prop_name,
> > + u32 **bw_vecs, int *len, u32 size)
> > +{
> > + int ret = 0;
> > + struct device_node *np = dev->of_node;
> > + size_t sz;
> > + u32 *arr = NULL;
> > +
> > + if (!of_get_property(np, prop_name, len)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + sz = *len = *len / sizeof(*arr);
> > + if (sz <= 0 || (size > 0 && (sz > size))) {
> > + dev_err(dev, "%s invalid size\n", prop_name);
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + arr = devm_kzalloc(dev, sz * sizeof(*arr), GFP_KERNEL);
> > + if (!arr) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + ret = of_property_read_u32_array(np, prop_name, arr, sz);
> > + if (ret < 0) {
> > + dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
> > + goto out;
> > + }
> > + *bw_vecs = arr;
> > +out:
> > + if (ret)
> > + *len = 0;
> > + return ret;
> > +}
> > +
> > +/* Returns required bandwidth in Bytes per Sec */
> > +static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
> > + struct mmc_ios *ios)
> > +{
> > + unsigned long bw;
> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> > +
> > + bw = msm_host->clk_rate;
> > +
> > + if (ios->bus_width == MMC_BUS_WIDTH_4)
> > + bw /= 2;
> > + else if (ios->bus_width == MMC_BUS_WIDTH_1)
> > + bw /= 8;
> > +
> > + return bw;
> > +}
> > +
> > +static int sdhci_msm_bus_get_vote_for_bw(struct sdhci_msm_host *host,
> > +unsigned int bw)
> > +{
> > + struct sdhci_msm_bus_vote_data *bvd = host->bus_vote_data;
> > +
> > + unsigned int *table = bvd->bw_vecs;
Should probably be a const bw_vecs pointer so that it can't be modified
after the fact.
> > + unsigned int size = bvd->bw_vecs_size;
> > + int i;
> > +
> > + for (i = 0; i < size; i++) {
> > + if (bw <= table[i])
> > + break;
return i;
> > + }
> > +
return i - 1;
> > + if (i && (i == size))
> > + i--;
> > +
> > + return i;
And then this is useless.
> > +}
> > +
> > +/*
> > + * This function must be called with host lock acquired.
> > + * Caller of this function should also ensure that msm bus client
> > + * handle is not null.
If it was NULL it would be pretty sad.
> > + */
> > +static inline int sdhci_msm_bus_set_vote(struct sdhci_msm_host
> > *msm_host,
> > + int vote,
> > + unsigned long *flags)
> > +{
> > + struct sdhci_host *host = platform_get_drvdata(msm_host->pdev);
> > + struct sdhci_msm_bus_vote_data *bvd = msm_host->bus_vote_data;
> > + struct msm_bus_path *usecase = bvd->usecase;
> > + struct msm_bus_vectors *vec = usecase[vote].vec;
> > + int ddr_rc = 0, cpu_rc
Re: [RFC 1/2] mmc: sdhci-msm: Add support for bus bandwidth voting
+Georgi Djakov
On 2019-09-06 18:17, Pradeep P V K wrote:
Vote for the MSM bus bandwidth required by SDHC driver
based on the clock frequency and bus width of the card.
Otherwise,the system clocks may run at minimum clock speed
and thus affecting the performance.
This change is based on Georgi Djakov [RFC]
(https://lkml.org/lkml/2018/10/11/499)
Signed-off-by: Sahitya Tummala
Signed-off-by: Subhash Jadavani
Signed-off-by: Veerabhadrarao Badiganti
Signed-off-by: Pradeep P V K
---
drivers/mmc/host/sdhci-msm.c | 393
++-
1 file changed, 390 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-msm.c
b/drivers/mmc/host/sdhci-msm.c
index b75c82d..71515ca 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -122,6 +123,9 @@
#define msm_host_writel(msm_host, val, host, offset) \
msm_host->var_ops->msm_writel_relaxed(val, host, offset)
+#define SDHC_DDR "sdhc-ddr"
+#define CPU_SDHC "cpu-sdhc"
+
struct sdhci_msm_offset {
u32 core_hc_mode;
u32 core_mci_data_cnt;
@@ -228,6 +232,31 @@ struct sdhci_msm_variant_info {
const struct sdhci_msm_offset *offset;
};
+struct msm_bus_vectors {
+ uint64_t ab;
+ uint64_t ib;
+};
+
+struct msm_bus_path {
+ unsigned int num_paths;
+ struct msm_bus_vectors *vec;
+};
+
+struct sdhci_msm_bus_vote_data {
+ const char *name;
+ unsigned int num_usecase;
+ struct msm_bus_path *usecase;
+
+ unsigned int *bw_vecs;
+ unsigned int bw_vecs_size;
+
+ struct icc_path *sdhc_ddr;
+ struct icc_path *cpu_sdhc;
+
+ uint32_t curr_vote;
+
+};
+
struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -253,8 +282,13 @@ struct sdhci_msm_host {
const struct sdhci_msm_offset *offset;
bool use_cdr;
u32 transfer_mode;
+ bool skip_bus_bw_voting;
+ struct sdhci_msm_bus_vote_data *bus_vote_data;
+ struct delayed_work bus_vote_work;
};
+static void sdhci_msm_bus_voting(struct sdhci_host *host, u32 enable);
+
static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct
sdhci_host *host)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1557,6 +1591,8 @@ static void sdhci_msm_set_clock(struct
sdhci_host *host, unsigned int clock)
msm_set_clock_rate_for_bus_mode(host, clock);
out:
+ if (!msm_host->skip_bus_bw_voting)
+ sdhci_msm_bus_voting(host, !!clock);
__sdhci_msm_set_clock(host, clock);
}
@@ -1678,6 +1714,341 @@ static void
sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
}
+static int sdhci_msm_dt_get_array(struct device *dev, const char
*prop_name,
+u32 **bw_vecs, int *len, u32 size)
+{
+ int ret = 0;
+ struct device_node *np = dev->of_node;
+ size_t sz;
+ u32 *arr = NULL;
+
+ if (!of_get_property(np, prop_name, len)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ sz = *len = *len / sizeof(*arr);
+ if (sz <= 0 || (size > 0 && (sz > size))) {
+ dev_err(dev, "%s invalid size\n", prop_name);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ arr = devm_kzalloc(dev, sz * sizeof(*arr), GFP_KERNEL);
+ if (!arr) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ ret = of_property_read_u32_array(np, prop_name, arr, sz);
+ if (ret < 0) {
+ dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
+ goto out;
+ }
+ *bw_vecs = arr;
+out:
+ if (ret)
+ *len = 0;
+ return ret;
+}
+
+/* Returns required bandwidth in Bytes per Sec */
+static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
+ struct mmc_ios *ios)
+{
+ unsigned long bw;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+ bw = msm_host->clk_rate;
+
+ if (ios->bus_width == MMC_BUS_WIDTH_4)
+ bw /= 2;
+ else if (ios->bus_width == MMC_BUS_WIDTH_1)
+ bw /= 8;
+
+ return bw;
+}
+
+static int sdhci_msm_bus_get_vote_for_bw(struct sdhci_msm_host *host,
+ unsigned int bw)
+{
+ struct sdhci_msm_bus_vote_data *bvd = host->bus_vote_data;
+
+ unsigned int *table = bvd->bw_vecs;
+ unsigned int size = bvd->bw_vecs_size;
+ int i;
+
+ for (i = 0; i < size; i++) {
+ if (bw <= table[i])
+ break;
+ }
+
+ if (i && (i == size))
+ i--;
+
+ return i;
+}
+
+/*
+ *
