Re: [PATCHÂV3 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support

[ppvk]

Hi Sibi,

Thanks for the review.

On 2020-06-05 17:10, Sibi Sankar wrote:
> Hey Pradeep,
> Thanks for the patch.
>
> On 2020-06-05 15:00, Pradeep P V K wrote:
> > Interconnect bandwidth scaling support is now added as a
> > part of OPP [1]. So, make sure interconnect driver is ready
>
> can you please replace driver with paths
> instead?
ok. I will address this in my next patch set.

> > before handling interconnect scaling.
> >
> > This change is based on
> > [1] [Patch v8] Introduce OPP bandwidth bindings
> > (https://lkml.org/lkml/2020/5/12/493)
> >
> > [2] [Patch v3] mmc: sdhci-msm: Fix error handling
> > for dev_pm_opp_of_add_table()
> > (https://lkml.org/lkml/2020/5/5/491)
>
> sry didn't notice ^^ earlier
> you might want to place these
> comments and dependencies similar
> to the following patch.
> https://patchwork.kernel.org/patch/11573903/
ok. will modify in my next patch.

> > Signed-off-by: Pradeep P V K <ppvk@xxxxxxxxxxxxxx>
> > ---
> >  drivers/mmc/host/sdhci-msm.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-msm.c 
> > b/drivers/mmc/host/sdhci-msm.c
> > index b277dd7..a945e84 100644
> > --- a/drivers/mmc/host/sdhci-msm.c
> > +++ b/drivers/mmc/host/sdhci-msm.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/interconnect.h>
> >
> >  #include "sdhci-pltfm.h"
> >  #include "cqhci.h"
> > @@ -2070,6 +2071,13 @@ static int sdhci_msm_probe(struct 
> > platform_device *pdev)
> >  	}
> >  	msm_host->bulk_clks[0].clk = clk;
> >
> > +	/* Make sure that ICC driver is ready for interconnect bandwdith
>
> typo /s/bandwdith/bandwidth
>
> > +	 * scaling before registering the device for OPP.
> > +	 */
>
> /* Check for optional interconnect paths */
> Maybe using ^^ would suffice since
> that's what we are actually doing
sure. i will re-modify the comments as suggested ^^ in my next patch.

> Reviewed-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
>
> > +	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
> > +	if (ret)
> > +		goto bus_clk_disable;
> > +
> >  	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
> >  	if (IS_ERR(msm_host->opp_table)) {
> >  		ret = PTR_ERR(msm_host->opp_table);