| Message ID | 201306030059.03783.heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/03/2013 07:59 AM, Heiko Stübner wrote: > Cortex-A9 SoCs from Rockchip use a slightly modified variant of dw_mmc > controllers that seems to require the SDMMC_CMD_USE_HOLD_REG bit to > always be set. > > There also seem to be no other modifications (additional register etc) > present, so to keep the footprint low, add this small variant to the > pltfm driver. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/mmc/host/dw_mmc-pltfm.c | 48 +++++++++++++++++++++++++++----------- > 1 files changed, 34 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > index 0048da8..7d041b5 100644 > --- a/drivers/mmc/host/dw_mmc-pltfm.c > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -24,6 +24,16 @@ > > #include "dw_mmc.h" > > + > +static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr) How about using "dw_mci_pltfm_prepare_command()"? Maybe HOLD_REG could be used at other SoC. > +{ > + *cmdr |= SDMMC_CMD_USE_HOLD_REG; > +} > + > +static const struct dw_mci_drv_data rockchip_drv_data = { > + .prepare_command = dw_mci_rockchip_prepare_command, > +}; > + > int dw_mci_pltfm_register(struct platform_device *pdev, > const struct dw_mci_drv_data *drv_data) > { > @@ -63,20 +73,6 @@ int dw_mci_pltfm_register(struct platform_device *pdev, > } > EXPORT_SYMBOL_GPL(dw_mci_pltfm_register); > > -static int dw_mci_pltfm_probe(struct platform_device *pdev) > -{ > - return dw_mci_pltfm_register(pdev, NULL); > -} > - > -int dw_mci_pltfm_remove(struct platform_device *pdev) > -{ > - struct dw_mci *host = platform_get_drvdata(pdev); > - > - dw_mci_remove(host); > - return 0; > -} > -EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); > - > #ifdef CONFIG_PM_SLEEP > /* > * TODO: we should probably disable the clock to the card in the suspend path. > @@ -114,10 +110,34 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops); > > static const struct of_device_id dw_mci_pltfm_match[] = { > { .compatible = "snps,dw-mshc", }, > + { .compatible = "rockchip,cortex-a9-dw-mshc", > + .data = &rockchip_drv_data }, > {}, > }; > MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); > > +static int dw_mci_pltfm_probe(struct platform_device *pdev) > +{ > + const struct dw_mci_drv_data *drv_data = NULL; > + const struct of_device_id *match; > + > + if (pdev->dev.of_node) { > + match = of_match_node(dw_mci_pltfm_match, pdev->dev.of_node); > + drv_data = match->data; > + } > + > + return dw_mci_pltfm_register(pdev, drv_data); > +} > + > +int dw_mci_pltfm_remove(struct platform_device *pdev) > +{ > + struct dw_mci *host = platform_get_drvdata(pdev); > + > + dw_mci_remove(host); > + return 0; > +} > +EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); > + > static struct platform_driver dw_mci_pltfm_driver = { > .probe = dw_mci_pltfm_probe, > .remove = dw_mci_pltfm_remove, >
Am Dienstag, 4. Juni 2013, 06:06:39 schrieb Jaehoon Chung: > On 06/03/2013 07:59 AM, Heiko Stübner wrote: > > Cortex-A9 SoCs from Rockchip use a slightly modified variant of dw_mmc > > controllers that seems to require the SDMMC_CMD_USE_HOLD_REG bit to > > always be set. > > > > There also seem to be no other modifications (additional register etc) > > present, so to keep the footprint low, add this small variant to the > > pltfm driver. > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > --- > > > > drivers/mmc/host/dw_mmc-pltfm.c | 48 > > +++++++++++++++++++++++++++----------- 1 files changed, 34 > > insertions(+), 14 deletions(-) > > > > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c > > b/drivers/mmc/host/dw_mmc-pltfm.c index 0048da8..7d041b5 100644 > > --- a/drivers/mmc/host/dw_mmc-pltfm.c > > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > > @@ -24,6 +24,16 @@ > > > > #include "dw_mmc.h" > > > > + > > +static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 > > *cmdr) > > How about using "dw_mci_pltfm_prepare_command()"? > Maybe HOLD_REG could be used at other SoC. The problem I had when thinking about it is that every implementation using the HOLD_REG stuff does it differently ... on the Exynos variant it depends on the CLKSEL register value and on the SOCFPGA variant on other clock values. It's only on the Rockchip variant that it seems to needed all the time. So, doing it with a "dw_mci_pltfm_prepare_command()" would need a flag to signal that the implementation needs the HOLD_REG all the time, but we won't know yet if other implementations will have other constraints on its use - like only i special cases or such. So personally I would keep it as it is for now, until more platforms using the HOLD_REG come along to see some sort of pattern of its use?
On 06/03/13 7:59 AM, Heiko Stübner wrote: > Cortex-A9 SoCs from Rockchip use a slightly modified variant of dw_mmc > controllers that seems to require the SDMMC_CMD_USE_HOLD_REG bit to > always be set. > > There also seem to be no other modifications (additional register etc) > present, so to keep the footprint low, add this small variant to the > pltfm driver. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/mmc/host/dw_mmc-pltfm.c | 48 +++++++++++++++++++++++++++----------- > 1 files changed, 34 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > index 0048da8..7d041b5 100644 > --- a/drivers/mmc/host/dw_mmc-pltfm.c > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -24,6 +24,16 @@ > > #include "dw_mmc.h" > > + > +static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr) > +{ > + *cmdr |= SDMMC_CMD_USE_HOLD_REG; Currently, SDMMC_CMD_USE_HOLD_REG is defined in dw_mmc-exynos.c It should be moved to dw_mmc.h Thanks, Seungwon Jeon > +} > + > +static const struct dw_mci_drv_data rockchip_drv_data = { > + .prepare_command = dw_mci_rockchip_prepare_command, > +}; > + > int dw_mci_pltfm_register(struct platform_device *pdev, > const struct dw_mci_drv_data *drv_data) > { > @@ -63,20 +73,6 @@ int dw_mci_pltfm_register(struct platform_device *pdev, > } > EXPORT_SYMBOL_GPL(dw_mci_pltfm_register); > > -static int dw_mci_pltfm_probe(struct platform_device *pdev) > -{ > - return dw_mci_pltfm_register(pdev, NULL); > -} > - > -int dw_mci_pltfm_remove(struct platform_device *pdev) > -{ > - struct dw_mci *host = platform_get_drvdata(pdev); > - > - dw_mci_remove(host); > - return 0; > -} > -EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); > - > #ifdef CONFIG_PM_SLEEP > /* > * TODO: we should probably disable the clock to the card in the suspend path. > @@ -114,10 +110,34 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops); > > static const struct of_device_id dw_mci_pltfm_match[] = { > { .compatible = "snps,dw-mshc", }, > + { .compatible = "rockchip,cortex-a9-dw-mshc", > + .data = &rockchip_drv_data }, > {}, > }; > MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); > > +static int dw_mci_pltfm_probe(struct platform_device *pdev) > +{ > + const struct dw_mci_drv_data *drv_data = NULL; > + const struct of_device_id *match; > + > + if (pdev->dev.of_node) { > + match = of_match_node(dw_mci_pltfm_match, pdev->dev.of_node); > + drv_data = match->data; > + } > + > + return dw_mci_pltfm_register(pdev, drv_data); > +} > + > +int dw_mci_pltfm_remove(struct platform_device *pdev) > +{ > + struct dw_mci *host = platform_get_drvdata(pdev); > + > + dw_mci_remove(host); > + return 0; > +} > +EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); > + > static struct platform_driver dw_mci_pltfm_driver = { > .probe = dw_mci_pltfm_probe, > .remove = dw_mci_pltfm_remove, > -- > 1.7.2.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Mittwoch, 5. Juni 2013, 16:00:43 schrieb Seungwon Jeon: > On 06/03/13 7:59 AM, Heiko Stübner wrote: > > Cortex-A9 SoCs from Rockchip use a slightly modified variant of dw_mmc > > controllers that seems to require the SDMMC_CMD_USE_HOLD_REG bit to > > always be set. > > > > There also seem to be no other modifications (additional register etc) > > present, so to keep the footprint low, add this small variant to the > > pltfm driver. > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > --- > > > > drivers/mmc/host/dw_mmc-pltfm.c | 48 > > +++++++++++++++++++++++++++----------- 1 files changed, 34 > > insertions(+), 14 deletions(-) > > > > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c > > b/drivers/mmc/host/dw_mmc-pltfm.c index 0048da8..7d041b5 100644 > > --- a/drivers/mmc/host/dw_mmc-pltfm.c > > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > > @@ -24,6 +24,16 @@ > > > > #include "dw_mmc.h" > > > > + > > +static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 > > *cmdr) +{ > > + *cmdr |= SDMMC_CMD_USE_HOLD_REG; > > Currently, SDMMC_CMD_USE_HOLD_REG is defined in dw_mmc-exynos.c > It should be moved to dw_mmc.h It's already done in another patch "mmc: dw_mmc: Add support DW SD/MMC driver on SOCFPGA" from Dinh Nguyen <dinguyen@altera.com>, which I therefore named as dependency in the cover letter. Don't know if it's better to also include the move here again and let the merging person sort out the conflict?
On Mon, Jun 3, 2013 at 1:59 AM, Heiko Stübner <heiko@sntech.de> wrote: > Cortex-A9 SoCs from Rockchip use a slightly modified variant of dw_mmc > controllers that seems to require the SDMMC_CMD_USE_HOLD_REG bit to > always be set. > > There also seem to be no other modifications (additional register etc) > present, so to keep the footprint low, add this small variant to the > pltfm driver. Few comments below. > --- a/drivers/mmc/host/dw_mmc-pltfm.c > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -24,6 +24,16 @@ > > #include "dw_mmc.h" > > + No need to add extra empty line here. > +static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr) > @@ -63,20 +73,6 @@ int dw_mci_pltfm_register(struct platform_device *pdev, > } > EXPORT_SYMBOL_GPL(dw_mci_pltfm_register); > > -static int dw_mci_pltfm_probe(struct platform_device *pdev) > -{ > - return dw_mci_pltfm_register(pdev, NULL); > -} > - > -int dw_mci_pltfm_remove(struct platform_device *pdev) > -{ > - struct dw_mci *host = platform_get_drvdata(pdev); > - > - dw_mci_remove(host); > - return 0; > -} > -EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); > - If you want to move those through code, please do separate patch. -- With Best Regards, Andy Shevchenko
diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c index 0048da8..7d041b5 100644 --- a/drivers/mmc/host/dw_mmc-pltfm.c +++ b/drivers/mmc/host/dw_mmc-pltfm.c @@ -24,6 +24,16 @@ #include "dw_mmc.h" + +static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr) +{ + *cmdr |= SDMMC_CMD_USE_HOLD_REG; +} + +static const struct dw_mci_drv_data rockchip_drv_data = { + .prepare_command = dw_mci_rockchip_prepare_command, +}; + int dw_mci_pltfm_register(struct platform_device *pdev, const struct dw_mci_drv_data *drv_data) { @@ -63,20 +73,6 @@ int dw_mci_pltfm_register(struct platform_device *pdev, } EXPORT_SYMBOL_GPL(dw_mci_pltfm_register); -static int dw_mci_pltfm_probe(struct platform_device *pdev) -{ - return dw_mci_pltfm_register(pdev, NULL); -} - -int dw_mci_pltfm_remove(struct platform_device *pdev) -{ - struct dw_mci *host = platform_get_drvdata(pdev); - - dw_mci_remove(host); - return 0; -} -EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); - #ifdef CONFIG_PM_SLEEP /* * TODO: we should probably disable the clock to the card in the suspend path. @@ -114,10 +110,34 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops); static const struct of_device_id dw_mci_pltfm_match[] = { { .compatible = "snps,dw-mshc", }, + { .compatible = "rockchip,cortex-a9-dw-mshc", + .data = &rockchip_drv_data }, {}, }; MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); +static int dw_mci_pltfm_probe(struct platform_device *pdev) +{ + const struct dw_mci_drv_data *drv_data = NULL; + const struct of_device_id *match; + + if (pdev->dev.of_node) { + match = of_match_node(dw_mci_pltfm_match, pdev->dev.of_node); + drv_data = match->data; + } + + return dw_mci_pltfm_register(pdev, drv_data); +} + +int dw_mci_pltfm_remove(struct platform_device *pdev) +{ + struct dw_mci *host = platform_get_drvdata(pdev); + + dw_mci_remove(host); + return 0; +} +EXPORT_SYMBOL_GPL(dw_mci_pltfm_remove); + static struct platform_driver dw_mci_pltfm_driver = { .probe = dw_mci_pltfm_probe, .remove = dw_mci_pltfm_remove,
Cortex-A9 SoCs from Rockchip use a slightly modified variant of dw_mmc controllers that seems to require the SDMMC_CMD_USE_HOLD_REG bit to always be set. There also seem to be no other modifications (additional register etc) present, so to keep the footprint low, add this small variant to the pltfm driver. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/mmc/host/dw_mmc-pltfm.c | 48 +++++++++++++++++++++++++++----------- 1 files changed, 34 insertions(+), 14 deletions(-)