| Message ID | 20190128064531.3331-10-faiz_abbas@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Add Support for eMMC in AM65x-evm | expand |
On Mon, Jan 28, 2019 at 12:15:27PM +0530, Faiz Abbas wrote: > Make set_ios_post() return int to faciliate error handling in > platform drivers. > > Signed-off-by: Faiz Abbas <faiz4000@gmail.com> > --- > drivers/mmc/sdhci.c | 6 +++++- > drivers/mmc/xenon_sdhci.c | 4 +++- > include/sdhci.h | 2 +- > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c > index 635f5396c4..b7b7ff6f7d 100644 > --- a/drivers/mmc/sdhci.c > +++ b/drivers/mmc/sdhci.c > @@ -461,6 +461,7 @@ static int sdhci_set_ios(struct mmc *mmc) > #endif > u32 ctrl; > struct sdhci_host *host = mmc->priv; > + int ret; > > if (host->ops && host->ops->set_control_reg) > host->ops->set_control_reg(host); > @@ -500,8 +501,11 @@ static int sdhci_set_ios(struct mmc *mmc) > sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); > > /* If available, call the driver specific "post" set_ios() function */ > - if (host->ops && host->ops->set_ios_post) > + if (host->ops && host->ops->set_ios_post) { > host->ops->set_ios_post(host); > + if (ret) > + return ret; > + } > > return 0; > } Isn't something going to complain about either unused or uninitialized (or, both) variables? In fact, re-reading this and follow-up patches, I think you forgot to turn: host->ops->set_ios_post(host); in to: ret = host->ops->set_ios_post(host); above. And could probably simplfy the whole thing to: if (host->ops && host->ops->set_ios_post) return host->ops->set_ios_post(host); return 0; Or is there more to the function that I'm missing? That's just based on the patch context alone.
Hi Tom, On 30/01/19 7:50 AM, Tom Rini wrote: > On Mon, Jan 28, 2019 at 12:15:27PM +0530, Faiz Abbas wrote: > >> Make set_ios_post() return int to faciliate error handling in >> platform drivers. >> >> Signed-off-by: Faiz Abbas <faiz4000@gmail.com> >> --- >> drivers/mmc/sdhci.c | 6 +++++- >> drivers/mmc/xenon_sdhci.c | 4 +++- >> include/sdhci.h | 2 +- >> 3 files changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c >> index 635f5396c4..b7b7ff6f7d 100644 >> --- a/drivers/mmc/sdhci.c >> +++ b/drivers/mmc/sdhci.c >> @@ -461,6 +461,7 @@ static int sdhci_set_ios(struct mmc *mmc) >> #endif >> u32 ctrl; >> struct sdhci_host *host = mmc->priv; >> + int ret; >> >> if (host->ops && host->ops->set_control_reg) >> host->ops->set_control_reg(host); >> @@ -500,8 +501,11 @@ static int sdhci_set_ios(struct mmc *mmc) >> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); >> >> /* If available, call the driver specific "post" set_ios() function */ >> - if (host->ops && host->ops->set_ios_post) >> + if (host->ops && host->ops->set_ios_post) { >> host->ops->set_ios_post(host); >> + if (ret) >> + return ret; >> + } >> >> return 0; >> } > > Isn't something going to complain about either unused or uninitialized > (or, both) variables? In fact, re-reading this and follow-up patches, I > think you forgot to turn: > host->ops->set_ios_post(host); > in to: > ret = host->ops->set_ios_post(host); > above. And could probably simplfy the whole thing to: > if (host->ops && host->ops->set_ios_post) > return host->ops->set_ios_post(host); > > return 0; > > Or is there more to the function that I'm missing? That's just based on > the patch context alone. You're right. Missed the ret = during some refactoring. Will fix Thanks, Faiz
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 635f5396c4..b7b7ff6f7d 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -461,6 +461,7 @@ static int sdhci_set_ios(struct mmc *mmc) #endif u32 ctrl; struct sdhci_host *host = mmc->priv; + int ret; if (host->ops && host->ops->set_control_reg) host->ops->set_control_reg(host); @@ -500,8 +501,11 @@ static int sdhci_set_ios(struct mmc *mmc) sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); /* If available, call the driver specific "post" set_ios() function */ - if (host->ops && host->ops->set_ios_post) + if (host->ops && host->ops->set_ios_post) { host->ops->set_ios_post(host); + if (ret) + return ret; + } return 0; } diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c index b576511338..829b75683b 100644 --- a/drivers/mmc/xenon_sdhci.c +++ b/drivers/mmc/xenon_sdhci.c @@ -326,7 +326,7 @@ static void xenon_mask_cmd_conflict_err(struct sdhci_host *host) } /* Platform specific function for post set_ios configuration */ -static void xenon_sdhci_set_ios_post(struct sdhci_host *host) +static int xenon_sdhci_set_ios_post(struct sdhci_host *host) { struct xenon_sdhci_priv *priv = host->mmc->priv; uint speed = host->mmc->tran_speed; @@ -364,6 +364,8 @@ static void xenon_sdhci_set_ios_post(struct sdhci_host *host) /* Re-init the PHY */ xenon_mmc_phy_set(host); + + return 0; } /* Install a driver specific handler for post set_ios configuration */ diff --git a/include/sdhci.h b/include/sdhci.h index 6cbba8f57a..725520b0b4 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -246,7 +246,7 @@ struct sdhci_ops { #endif int (*get_cd)(struct sdhci_host *host); void (*set_control_reg)(struct sdhci_host *host); - void (*set_ios_post)(struct sdhci_host *host); + int (*set_ios_post)(struct sdhci_host *host); void (*set_clock)(struct sdhci_host *host, u32 div); int (*platform_execute_tuning)(struct mmc *host, u8 opcode); void (*set_delay)(struct sdhci_host *host);
Make set_ios_post() return int to faciliate error handling in platform drivers. Signed-off-by: Faiz Abbas <faiz4000@gmail.com> --- drivers/mmc/sdhci.c | 6 +++++- drivers/mmc/xenon_sdhci.c | 4 +++- include/sdhci.h | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-)