2017-11-20 4:49 GMT+09:00 Wolfram Sang <wsa@xxxxxxxxxxxxx>: > On Tue, Nov 07, 2017 at 05:09:28PM +0900, Masahiro Yamada wrote: >> The remove, suspend, resume hooks need tmio_mmc_host. It is tedious >> to get mmc_host from the driver_data and pass it to mmc_priv(). >> We can directly set tmio_mmc_host to driver data to clean up the code. >> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> > > ... > >> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c >> index 64b7e9f..ccfbc15 100644 >> --- a/drivers/mmc/host/tmio_mmc.c >> +++ b/drivers/mmc/host/tmio_mmc.c >> @@ -128,15 +128,11 @@ static int tmio_mmc_probe(struct platform_device *pdev) >> static int tmio_mmc_remove(struct platform_device *pdev) >> { >> const struct mfd_cell *cell = mfd_get_cell(pdev); >> - struct mmc_host *mmc = platform_get_drvdata(pdev); >> + struct tmio_mmc_host *host = platform_get_drvdata(pdev); >> >> - if (mmc) { >> - struct tmio_mmc_host *host = mmc_priv(mmc); >> - >> - tmio_mmc_host_remove(host); >> - if (cell->disable) >> - cell->disable(pdev); >> - } >> + tmio_mmc_host_remove(host); >> + if (cell->disable) >> + cell->disable(pdev); > > Hmmm, this changes the code logic. Any reason this driver checks for a > valid 'mmc' and can we safely drop it? > This code has been here since the initial support of TMIO by commit 4a48998fa16121d0fe3436cce43afd6f47424103. So, we have no way to know the reason except asking the author, Ian Molton. My best guess is unnecessary if-conditional was added and overlooked in the review process. mmc has been allocated, and platform_set_drvdata() has been called in the driver probe. I do not see any case for mmc==NULL. -- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html