| Message ID | 20250115073026.31552-2-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: mmc: mtk-sd: Add eMMC for AN7581 | expand |
On Wed, 2025-01-15 at 08:29 +0100, Christian Marangi wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Add support for AN7581 MMC Host. The MMC Host controller is based on > mt7622 with the difference of not having regulator supply and > state_uhs > pins and hclk clock. > > Some minor fixes are applied to check if the state_uhs pins are > defined > and make hclk optional for the new airoha compatible. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++----- > -- > 1 file changed, 46 insertions(+), 9 deletions(-) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index efb0d2d5716b..9d6868883c91 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible > mt8196_compat = { > .support_new_rx = true, > }; > > +static const struct mtk_mmc_compatible an7581_compat = { > + .clk_div_bits = 12, > + .recheck_sdio_irq = true, > + .hs400_tune = false, > + .pad_tune_reg = MSDC_PAD_TUNE0, > + .async_fifo = true, > + .data_tune = true, > + .busy_check = true, > + .stop_clk_fix = true, > + .stop_dly_sel = 3, > + .enhance_rx = true, > + .support_64g = false, > +}; > + > static const struct of_device_id msdc_of_ids[] = { > { .compatible = "mediatek,mt2701-mmc", .data = > &mt2701_compat}, > { .compatible = "mediatek,mt2712-mmc", .data = > &mt2712_compat}, > @@ -680,7 +694,7 @@ static const struct of_device_id msdc_of_ids[] = > { > { .compatible = "mediatek,mt8183-mmc", .data = > &mt8183_compat}, > { .compatible = "mediatek,mt8196-mmc", .data = > &mt8196_compat}, > { .compatible = "mediatek,mt8516-mmc", .data = > &mt8516_compat}, > - > + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, > {} > }; > MODULE_DEVICE_TABLE(of, msdc_of_ids); > @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct > mmc_host *mmc, struct mmc_ios *ios) > struct msdc_host *host = mmc_priv(mmc); > int ret; > > + /* Skip setting supply if not supported */ > + if (!mmc->supply.vqmmc) > + return 0; > + > if (!IS_ERR(mmc->supply.vqmmc)) { > if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && > ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct > mmc_host *mmc, int enb) > dev_dbg(host->dev, "SDIO eint irq: > %d!\n", host->eint_irq); > } > > - pinctrl_select_state(host->pinctrl, host- > >pins_uhs); > + /* Skip setting uhs pins if not supported */ > + if (host->pins_uhs) > + pinctrl_select_state(host->pinctrl, > host->pins_uhs); > } else { > dev_pm_clear_wake_irq(host->dev); > } > @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct mmc_host > *mmc, struct mmc_ios *ios) > > msdc_set_buswidth(host, ios->bus_width); > > + /* Skip regulator if not supported */ > + if (!mmc->supply.vmmc) > + goto skip_regulator; > + If power_mode is MMC_POWER_UP, we need to execute the related flow. Now it will skip mmc_init_hw(host), which will cause problems. BTW, can this be implemented using a fix regulator? if ok, no need to modify here. > /* Suspend/Resume will do power off/on */ > switch (ios->power_mode) { > case MMC_POWER_UP: > @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct mmc_host > *mmc, struct mmc_ios *ios) > break; > } > > +skip_regulator: > if (host->mclk != ios->clock || host->timing != ios->timing) > msdc_set_mclk(host, ios->timing, ios->clock); > } > @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct > platform_device *pdev, > if (IS_ERR(host->src_clk)) > return PTR_ERR(host->src_clk); > > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > - if (IS_ERR(host->h_clk)) > - return PTR_ERR(host->h_clk); > + /* AN7581 SoC doesn't have hclk */ > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > + if (IS_ERR(host->h_clk)) > + return PTR_ERR(host->h_clk); > + } > > host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > if (IS_ERR(host->bus_clk)) > @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct > platform_device *pdev) > return PTR_ERR(host->pins_default); > } > > - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > "state_uhs"); > - if (IS_ERR(host->pins_uhs)) { > - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); > - return PTR_ERR(host->pins_uhs); > + /* AN7581 doesn't have state_uhs pins */ > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > "state_uhs"); > + if (IS_ERR(host->pins_uhs)) { > + dev_err(&pdev->dev, "Cannot find pinctrl > uhs!\n"); > + return PTR_ERR(host->pins_uhs); > + } > } > > /* Support for SDIO eint irq ? */ > @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct > platform_device *pdev) > dev_err(&pdev->dev, "Cannot ungate clocks!\n"); > goto release_clk; > } > + > + /* AN7581 without regulator require tune to OCR values */ > + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && > + !mmc->ocr_avail) > + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; > + > msdc_init_hw(host); > > if (mmc->caps2 & MMC_CAP2_CQE) { > -- > 2.45.2 >
On Wed, Jan 15, 2025 at 09:33:35AM +0000, Wenbin Mei (梅文彬) wrote: > On Wed, 2025-01-15 at 08:29 +0100, Christian Marangi wrote: > > External email : Please do not click links or open attachments until > > you have verified the sender or the content. > > > > > > Add support for AN7581 MMC Host. The MMC Host controller is based on > > mt7622 with the difference of not having regulator supply and > > state_uhs > > pins and hclk clock. > > > > Some minor fixes are applied to check if the state_uhs pins are > > defined > > and make hclk optional for the new airoha compatible. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++----- > > -- > > 1 file changed, 46 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > > index efb0d2d5716b..9d6868883c91 100644 > > --- a/drivers/mmc/host/mtk-sd.c > > +++ b/drivers/mmc/host/mtk-sd.c > > @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible > > mt8196_compat = { > > .support_new_rx = true, > > }; > > > > +static const struct mtk_mmc_compatible an7581_compat = { > > + .clk_div_bits = 12, > > + .recheck_sdio_irq = true, > > + .hs400_tune = false, > > + .pad_tune_reg = MSDC_PAD_TUNE0, > > + .async_fifo = true, > > + .data_tune = true, > > + .busy_check = true, > > + .stop_clk_fix = true, > > + .stop_dly_sel = 3, > > + .enhance_rx = true, > > + .support_64g = false, > > +}; > > + > > static const struct of_device_id msdc_of_ids[] = { > > { .compatible = "mediatek,mt2701-mmc", .data = > > &mt2701_compat}, > > { .compatible = "mediatek,mt2712-mmc", .data = > > &mt2712_compat}, > > @@ -680,7 +694,7 @@ static const struct of_device_id msdc_of_ids[] = > > { > > { .compatible = "mediatek,mt8183-mmc", .data = > > &mt8183_compat}, > > { .compatible = "mediatek,mt8196-mmc", .data = > > &mt8196_compat}, > > { .compatible = "mediatek,mt8516-mmc", .data = > > &mt8516_compat}, > > - > > + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, > > {} > > }; > > MODULE_DEVICE_TABLE(of, msdc_of_ids); > > @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct > > mmc_host *mmc, struct mmc_ios *ios) > > struct msdc_host *host = mmc_priv(mmc); > > int ret; > > > > + /* Skip setting supply if not supported */ > > + if (!mmc->supply.vqmmc) > > + return 0; > > + > > if (!IS_ERR(mmc->supply.vqmmc)) { > > if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && > > ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > > @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct > > mmc_host *mmc, int enb) > > dev_dbg(host->dev, "SDIO eint irq: > > %d!\n", host->eint_irq); > > } > > > > - pinctrl_select_state(host->pinctrl, host- > > >pins_uhs); > > + /* Skip setting uhs pins if not supported */ > > + if (host->pins_uhs) > > + pinctrl_select_state(host->pinctrl, > > host->pins_uhs); > > } else { > > dev_pm_clear_wake_irq(host->dev); > > } > > @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct mmc_host > > *mmc, struct mmc_ios *ios) > > > > msdc_set_buswidth(host, ios->bus_width); > > > > + /* Skip regulator if not supported */ > > + if (!mmc->supply.vmmc) > > + goto skip_regulator; > > + > If power_mode is MMC_POWER_UP, we need to execute the related flow. > Now it will skip mmc_init_hw(host), which will cause problems. > BTW, can this be implemented using a fix regulator? if ok, no need > to modify here. Yes I didn't think of that, I will test if this is possible and reply back, thanks for the hint. > > /* Suspend/Resume will do power off/on */ > > switch (ios->power_mode) { > > case MMC_POWER_UP: > > @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct mmc_host > > *mmc, struct mmc_ios *ios) > > break; > > } > > > > +skip_regulator: > > if (host->mclk != ios->clock || host->timing != ios->timing) > > msdc_set_mclk(host, ios->timing, ios->clock); > > } > > @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct > > platform_device *pdev, > > if (IS_ERR(host->src_clk)) > > return PTR_ERR(host->src_clk); > > > > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > - if (IS_ERR(host->h_clk)) > > - return PTR_ERR(host->h_clk); > > + /* AN7581 SoC doesn't have hclk */ > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > + if (IS_ERR(host->h_clk)) > > + return PTR_ERR(host->h_clk); > > + } > > > > host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > > if (IS_ERR(host->bus_clk)) > > @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct > > platform_device *pdev) > > return PTR_ERR(host->pins_default); > > } > > > > - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > "state_uhs"); > > - if (IS_ERR(host->pins_uhs)) { > > - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); > > - return PTR_ERR(host->pins_uhs); > > + /* AN7581 doesn't have state_uhs pins */ > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > "state_uhs"); > > + if (IS_ERR(host->pins_uhs)) { > > + dev_err(&pdev->dev, "Cannot find pinctrl > > uhs!\n"); > > + return PTR_ERR(host->pins_uhs); > > + } > > } > > > > /* Support for SDIO eint irq ? */ > > @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct > > platform_device *pdev) > > dev_err(&pdev->dev, "Cannot ungate clocks!\n"); > > goto release_clk; > > } > > + > > + /* AN7581 without regulator require tune to OCR values */ > > + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && > > + !mmc->ocr_avail) > > + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; > > + > > msdc_init_hw(host); > > > > if (mmc->caps2 & MMC_CAP2_CQE) { > > -- > > 2.45.2 > >
On Wed, 2025-01-15 at 08:29 +0100, Christian Marangi wrote: > Add support for AN7581 MMC Host. The MMC Host controller is based on > mt7622 with the difference of not having regulator supply and > state_uhs > pins and hclk clock. > > Some minor fixes are applied to check if the state_uhs pins are > defined > and make hclk optional for the new airoha compatible. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++----- > -- > 1 file changed, 46 insertions(+), 9 deletions(-) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index efb0d2d5716b..9d6868883c91 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible > mt8196_compat = { > .support_new_rx = true, > }; > > +static const struct mtk_mmc_compatible an7581_compat = { > + .clk_div_bits = 12, > + .recheck_sdio_irq = true, > + .hs400_tune = false, > + .pad_tune_reg = MSDC_PAD_TUNE0, > + .async_fifo = true, > + .data_tune = true, > + .busy_check = true, > + .stop_clk_fix = true, > + .stop_dly_sel = 3, > + .enhance_rx = true, > + .support_64g = false, > +}; > + > static const struct of_device_id msdc_of_ids[] = { > { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, > { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, > @@ -680,7 +694,7 @@ static const struct of_device_id msdc_of_ids[] = > { > { .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat}, > { .compatible = "mediatek,mt8196-mmc", .data = &mt8196_compat}, > { .compatible = "mediatek,mt8516-mmc", .data = &mt8516_compat}, > - > + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, > {} > }; > MODULE_DEVICE_TABLE(of, msdc_of_ids); > @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct > mmc_host *mmc, struct mmc_ios *ios) > struct msdc_host *host = mmc_priv(mmc); > int ret; > > + /* Skip setting supply if not supported */ > + if (!mmc->supply.vqmmc) > + return 0; > + Hi Christian, I think here is no need. If you have not 'vqmmc' in the dts, IS_ERR(mmc->supply.vqmmc) would be -ENODEV and the corresponding flow would not be executed. And another question, host->pins_default is just selected here, that would be lost. > if (!IS_ERR(mmc->supply.vqmmc)) { > if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && > ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct > mmc_host *mmc, int enb) > dev_dbg(host->dev, "SDIO eint irq: > %d!\n", host->eint_irq); > } > > - pinctrl_select_state(host->pinctrl, host- > >pins_uhs); > + /* Skip setting uhs pins if not supported */ > + if (host->pins_uhs) > + pinctrl_select_state(host->pinctrl, > host->pins_uhs); > } else { > dev_pm_clear_wake_irq(host->dev); > } > @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct mmc_host > *mmc, struct mmc_ios *ios) > > msdc_set_buswidth(host, ios->bus_width); > > + /* Skip regulator if not supported */ > + if (!mmc->supply.vmmc) > + goto skip_regulator; > + No need too. > /* Suspend/Resume will do power off/on */ > switch (ios->power_mode) { > case MMC_POWER_UP: > @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct mmc_host > *mmc, struct mmc_ios *ios) > break; > } > > +skip_regulator: > if (host->mclk != ios->clock || host->timing != ios->timing) > msdc_set_mclk(host, ios->timing, ios->clock); > } > @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct > platform_device *pdev, > if (IS_ERR(host->src_clk)) > return PTR_ERR(host->src_clk); > > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > - if (IS_ERR(host->h_clk)) > - return PTR_ERR(host->h_clk); > + /* AN7581 SoC doesn't have hclk */ > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > + if (IS_ERR(host->h_clk)) > + return PTR_ERR(host->h_clk); > + } devm_clk_get_optional could be used to instead here, no need to use compatible to distinguish. > host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > if (IS_ERR(host->bus_clk)) > @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct > platform_device *pdev) > return PTR_ERR(host->pins_default); > } > > - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > "state_uhs"); > - if (IS_ERR(host->pins_uhs)) { > - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); > - return PTR_ERR(host->pins_uhs); > + /* AN7581 doesn't have state_uhs pins */ > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > "state_uhs"); > + if (IS_ERR(host->pins_uhs)) { > + dev_err(&pdev->dev, "Cannot find pinctrl > uhs!\n"); > + return PTR_ERR(host->pins_uhs); > + } > } Could you consider to set a dummy 'state_uhs' same as 'state_default' in the dts, that you could not use compatible to distinguish here. > > /* Support for SDIO eint irq ? */ > @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct > platform_device *pdev) > dev_err(&pdev->dev, "Cannot ungate clocks!\n"); > goto release_clk; > } > + > + /* AN7581 without regulator require tune to OCR values */ > + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && > + !mmc->ocr_avail) > + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; > + Maybe you could use regulator-fixed in the dts and configure min/max voltage to get ocr_avail, no need to set hard code here. > msdc_init_hw(host); > > if (mmc->caps2 & MMC_CAP2_CQE) {
On Thu, Jan 16, 2025 at 07:01:13AM +0000, Andy-ld Lu (卢东) wrote: > On Wed, 2025-01-15 at 08:29 +0100, Christian Marangi wrote: > > Add support for AN7581 MMC Host. The MMC Host controller is based on > > mt7622 with the difference of not having regulator supply and > > state_uhs > > pins and hclk clock. > > > > Some minor fixes are applied to check if the state_uhs pins are > > defined > > and make hclk optional for the new airoha compatible. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++----- > > -- > > 1 file changed, 46 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > > index efb0d2d5716b..9d6868883c91 100644 > > --- a/drivers/mmc/host/mtk-sd.c > > +++ b/drivers/mmc/host/mtk-sd.c > > @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible > > mt8196_compat = { > > .support_new_rx = true, > > }; > > > > +static const struct mtk_mmc_compatible an7581_compat = { > > + .clk_div_bits = 12, > > + .recheck_sdio_irq = true, > > + .hs400_tune = false, > > + .pad_tune_reg = MSDC_PAD_TUNE0, > > + .async_fifo = true, > > + .data_tune = true, > > + .busy_check = true, > > + .stop_clk_fix = true, > > + .stop_dly_sel = 3, > > + .enhance_rx = true, > > + .support_64g = false, > > +}; > > + > > static const struct of_device_id msdc_of_ids[] = { > > { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, > > { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, > > @@ -680,7 +694,7 @@ static const struct of_device_id msdc_of_ids[] = > > { > > { .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat}, > > { .compatible = "mediatek,mt8196-mmc", .data = &mt8196_compat}, > > { .compatible = "mediatek,mt8516-mmc", .data = &mt8516_compat}, > > - > > + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, > > {} > > }; > > MODULE_DEVICE_TABLE(of, msdc_of_ids); > > @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct > > mmc_host *mmc, struct mmc_ios *ios) > > struct msdc_host *host = mmc_priv(mmc); > > int ret; > > > > + /* Skip setting supply if not supported */ > > + if (!mmc->supply.vqmmc) > > + return 0; > > + > Hi Christian, > > I think here is no need. If you have not 'vqmmc' in the > dts, IS_ERR(mmc->supply.vqmmc) would be -ENODEV and the corresponding > flow would not be executed. > > And another question, host->pins_default is just selected here, that > would be lost. > > > if (!IS_ERR(mmc->supply.vqmmc)) { > > if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && > > ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > > @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct > > mmc_host *mmc, int enb) > > dev_dbg(host->dev, "SDIO eint irq: > > %d!\n", host->eint_irq); > > } > > > > - pinctrl_select_state(host->pinctrl, host- > > >pins_uhs); > > + /* Skip setting uhs pins if not supported */ > > + if (host->pins_uhs) > > + pinctrl_select_state(host->pinctrl, > > host->pins_uhs); > > } else { > > dev_pm_clear_wake_irq(host->dev); > > } > > @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct mmc_host > > *mmc, struct mmc_ios *ios) > > > > msdc_set_buswidth(host, ios->bus_width); > > > > + /* Skip regulator if not supported */ > > + if (!mmc->supply.vmmc) > > + goto skip_regulator; > > + > No need too. > > > /* Suspend/Resume will do power off/on */ > > switch (ios->power_mode) { > > case MMC_POWER_UP: > > @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct mmc_host > > *mmc, struct mmc_ios *ios) > > break; > > } > > > > +skip_regulator: > > if (host->mclk != ios->clock || host->timing != ios->timing) > > msdc_set_mclk(host, ios->timing, ios->clock); > > } > > @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct > > platform_device *pdev, > > if (IS_ERR(host->src_clk)) > > return PTR_ERR(host->src_clk); > > > > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > - if (IS_ERR(host->h_clk)) > > - return PTR_ERR(host->h_clk); > > + /* AN7581 SoC doesn't have hclk */ > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > + if (IS_ERR(host->h_clk)) > > + return PTR_ERR(host->h_clk); > > + } > devm_clk_get_optional could be used to instead here, no need to use > compatible to distinguish. > I can make the hclk optional but I think this would affect also every other compatible by hiding broken clock configuration. > > host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > > if (IS_ERR(host->bus_clk)) > > @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct > > platform_device *pdev) > > return PTR_ERR(host->pins_default); > > } > > > > - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > "state_uhs"); > > - if (IS_ERR(host->pins_uhs)) { > > - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); > > - return PTR_ERR(host->pins_uhs); > > + /* AN7581 doesn't have state_uhs pins */ > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > "state_uhs"); > > + if (IS_ERR(host->pins_uhs)) { > > + dev_err(&pdev->dev, "Cannot find pinctrl > > uhs!\n"); > > + return PTR_ERR(host->pins_uhs); > > + } > > } > Could you consider to set a dummy 'state_uhs' same as 'state_default' > in the dts, that you could not use compatible to distinguish here. > This is problematic, correct me if I'm wrong, you are suggesting to assign the emmc pins to both default and uhs? This is problematic as the pinctrl driver would complain that such pins are already assigned to something. Also I don't think it's possible to assign these pins to a dummy pin. > > > > /* Support for SDIO eint irq ? */ > > @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct > > platform_device *pdev) > > dev_err(&pdev->dev, "Cannot ungate clocks!\n"); > > goto release_clk; > > } > > + > > + /* AN7581 without regulator require tune to OCR values */ > > + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && > > + !mmc->ocr_avail) > > + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; > > + > Maybe you could use regulator-fixed in the dts and configure min/max > voltage to get ocr_avail, no need to set hard code here. > Also suggested by Wenbin Mei (梅文彬) and I just tried this. This can't be done, fixed-regulator needs to have the same min and max voltage or they fail to probe sooo fixed-regulator saddly can't be used :( I will send a new version of this with the other point corrected but I think a compatible and these additional if is a must :( > > msdc_init_hw(host); > > > > if (mmc->caps2 & MMC_CAP2_CQE) {
On Mon, 2025-01-20 at 18:29 +0100, Christian Marangi wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On Thu, Jan 16, 2025 at 07:01:13AM +0000, Andy-ld Lu (卢东) wrote: > > On Wed, 2025-01-15 at 08:29 +0100, Christian Marangi wrote: > > > Add support for AN7581 MMC Host. The MMC Host controller is based > > > on > > > mt7622 with the difference of not having regulator supply and > > > state_uhs > > > pins and hclk clock. > > > > > > Some minor fixes are applied to check if the state_uhs pins are > > > defined > > > and make hclk optional for the new airoha compatible. > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > --- > > > drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++- > > > ---- > > > -- > > > 1 file changed, 46 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk- > > > sd.c > > > index efb0d2d5716b..9d6868883c91 100644 > > > --- a/drivers/mmc/host/mtk-sd.c > > > +++ b/drivers/mmc/host/mtk-sd.c > > > @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible > > > mt8196_compat = { > > > .support_new_rx = true, > > > }; > > > > > > +static const struct mtk_mmc_compatible an7581_compat = { > > > + .clk_div_bits = 12, > > > + .recheck_sdio_irq = true, > > > + .hs400_tune = false, > > > + .pad_tune_reg = MSDC_PAD_TUNE0, > > > + .async_fifo = true, > > > + .data_tune = true, > > > + .busy_check = true, > > > + .stop_clk_fix = true, > > > + .stop_dly_sel = 3, > > > + .enhance_rx = true, > > > + .support_64g = false, > > > +}; > > > + > > > static const struct of_device_id msdc_of_ids[] = { > > > { .compatible = "mediatek,mt2701-mmc", .data = > > > &mt2701_compat}, > > > { .compatible = "mediatek,mt2712-mmc", .data = > > > &mt2712_compat}, > > > @@ -680,7 +694,7 @@ static const struct of_device_id > > > msdc_of_ids[] = > > > { > > > { .compatible = "mediatek,mt8183-mmc", .data = > > > &mt8183_compat}, > > > { .compatible = "mediatek,mt8196-mmc", .data = > > > &mt8196_compat}, > > > { .compatible = "mediatek,mt8516-mmc", .data = > > > &mt8516_compat}, > > > - > > > + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, > > > {} > > > }; > > > MODULE_DEVICE_TABLE(of, msdc_of_ids); > > > @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct > > > mmc_host *mmc, struct mmc_ios *ios) > > > struct msdc_host *host = mmc_priv(mmc); > > > int ret; > > > > > > + /* Skip setting supply if not supported */ > > > + if (!mmc->supply.vqmmc) > > > + return 0; > > > + > > > > Hi Christian, > > > > I think here is no need. If you have not 'vqmmc' in the > > dts, IS_ERR(mmc->supply.vqmmc) would be -ENODEV and the > > corresponding > > flow would not be executed. > > > > And another question, host->pins_default is just selected here, > > that > > would be lost. > > > > > if (!IS_ERR(mmc->supply.vqmmc)) { > > > if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && > > > ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > > > @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct > > > mmc_host *mmc, int enb) > > > dev_dbg(host->dev, "SDIO eint irq: > > > %d!\n", host->eint_irq); > > > } > > > > > > - pinctrl_select_state(host->pinctrl, host- > > > > pins_uhs); > > > > > > + /* Skip setting uhs pins if not supported */ > > > + if (host->pins_uhs) > > > + pinctrl_select_state(host->pinctrl, > > > host->pins_uhs); > > > } else { > > > dev_pm_clear_wake_irq(host->dev); > > > } > > > @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct > > > mmc_host > > > *mmc, struct mmc_ios *ios) > > > > > > msdc_set_buswidth(host, ios->bus_width); > > > > > > + /* Skip regulator if not supported */ > > > + if (!mmc->supply.vmmc) > > > + goto skip_regulator; > > > + > > > > No need too. > > > > > /* Suspend/Resume will do power off/on */ > > > switch (ios->power_mode) { > > > case MMC_POWER_UP: > > > @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct > > > mmc_host > > > *mmc, struct mmc_ios *ios) > > > break; > > > } > > > > > > +skip_regulator: > > > if (host->mclk != ios->clock || host->timing != ios->timing) > > > msdc_set_mclk(host, ios->timing, ios->clock); > > > } > > > @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct > > > platform_device *pdev, > > > if (IS_ERR(host->src_clk)) > > > return PTR_ERR(host->src_clk); > > > > > > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > > - if (IS_ERR(host->h_clk)) > > > - return PTR_ERR(host->h_clk); > > > + /* AN7581 SoC doesn't have hclk */ > > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > > + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > > + if (IS_ERR(host->h_clk)) > > > + return PTR_ERR(host->h_clk); > > > + } > > > > devm_clk_get_optional could be used to instead here, no need to use > > compatible to distinguish. > > > > I can make the hclk optional but I think this would affect also every > other compatible by hiding broken clock configuration. > > > > host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > > > if (IS_ERR(host->bus_clk)) > > > @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct > > > platform_device *pdev) > > > return PTR_ERR(host->pins_default); > > > } > > > > > > - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > > "state_uhs"); > > > - if (IS_ERR(host->pins_uhs)) { > > > - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); > > > - return PTR_ERR(host->pins_uhs); > > > + /* AN7581 doesn't have state_uhs pins */ > > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > > + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > > "state_uhs"); > > > + if (IS_ERR(host->pins_uhs)) { > > > + dev_err(&pdev->dev, "Cannot find pinctrl > > > uhs!\n"); > > > + return PTR_ERR(host->pins_uhs); > > > + } > > > } > > > > Could you consider to set a dummy 'state_uhs' same as > > 'state_default' > > in the dts, that you could not use compatible to distinguish here. > > > > This is problematic, correct me if I'm wrong, you are suggesting to > assign the emmc pins to both default and uhs? This is problematic as > the > pinctrl driver would complain that such pins are already assigned to > something. Also I don't think it's possible to assign these pins to a > dummy pin. > Maybe I have not expressed clearly...What I mean is that you could set as below, and the content of &mmc_pins_uhs is just copied from &mmc_pins_default. mmc@1fa0e000 { ... pinctrl-names = "default", "state_uhs"; pinctrl-0 = <&mmc_pins_default>; pinctrl-1 = <&mmc_pins_uhs>; } > > > > > > /* Support for SDIO eint irq ? */ > > > @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct > > > platform_device *pdev) > > > dev_err(&pdev->dev, "Cannot ungate clocks!\n"); > > > goto release_clk; > > > } > > > + > > > + /* AN7581 without regulator require tune to OCR values */ > > > + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && > > > + !mmc->ocr_avail) > > > + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; > > > + > > > > Maybe you could use regulator-fixed in the dts and configure > > min/max > > voltage to get ocr_avail, no need to set hard code here. > > > > Also suggested by Wenbin Mei (梅文彬) and I just tried this. > > This can't be done, fixed-regulator needs to have the same min and > max > voltage or they fail to probe sooo fixed-regulator saddly can't be > used > :( > > I will send a new version of this with the other point corrected but > I > think a compatible and these additional if is a must :( If use the fixed regulator such as below, you will get the same ocr_avail as 'MMC_VDD_32_33 | MMC_VDD_33_34' through mmc_regulator_get_ocrmask(). vmmc_3v3: regulator-vmmc-3v3 { compatible = "regulator-fixed"; regulator-name = "vmmc"; regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-always-on; } > > > > msdc_init_hw(host); > > > > > > if (mmc->caps2 & MMC_CAP2_CQE) { > > -- > Ansuel
On Tue, Jan 21, 2025 at 06:25:48AM +0000, Andy-ld Lu (卢东) wrote: > On Mon, 2025-01-20 at 18:29 +0100, Christian Marangi wrote: > > External email : Please do not click links or open attachments until > > you have verified the sender or the content. > > > > > > On Thu, Jan 16, 2025 at 07:01:13AM +0000, Andy-ld Lu (卢东) wrote: > > > On Wed, 2025-01-15 at 08:29 +0100, Christian Marangi wrote: > > > > Add support for AN7581 MMC Host. The MMC Host controller is based > > > > on > > > > mt7622 with the difference of not having regulator supply and > > > > state_uhs > > > > pins and hclk clock. > > > > > > > > Some minor fixes are applied to check if the state_uhs pins are > > > > defined > > > > and make hclk optional for the new airoha compatible. > > > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > > --- > > > > drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++- > > > > ---- > > > > -- > > > > 1 file changed, 46 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk- > > > > sd.c > > > > index efb0d2d5716b..9d6868883c91 100644 > > > > --- a/drivers/mmc/host/mtk-sd.c > > > > +++ b/drivers/mmc/host/mtk-sd.c > > > > @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible > > > > mt8196_compat = { > > > > .support_new_rx = true, > > > > }; > > > > > > > > +static const struct mtk_mmc_compatible an7581_compat = { > > > > + .clk_div_bits = 12, > > > > + .recheck_sdio_irq = true, > > > > + .hs400_tune = false, > > > > + .pad_tune_reg = MSDC_PAD_TUNE0, > > > > + .async_fifo = true, > > > > + .data_tune = true, > > > > + .busy_check = true, > > > > + .stop_clk_fix = true, > > > > + .stop_dly_sel = 3, > > > > + .enhance_rx = true, > > > > + .support_64g = false, > > > > +}; > > > > + > > > > static const struct of_device_id msdc_of_ids[] = { > > > > { .compatible = "mediatek,mt2701-mmc", .data = > > > > &mt2701_compat}, > > > > { .compatible = "mediatek,mt2712-mmc", .data = > > > > &mt2712_compat}, > > > > @@ -680,7 +694,7 @@ static const struct of_device_id > > > > msdc_of_ids[] = > > > > { > > > > { .compatible = "mediatek,mt8183-mmc", .data = > > > > &mt8183_compat}, > > > > { .compatible = "mediatek,mt8196-mmc", .data = > > > > &mt8196_compat}, > > > > { .compatible = "mediatek,mt8516-mmc", .data = > > > > &mt8516_compat}, > > > > - > > > > + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, > > > > {} > > > > }; > > > > MODULE_DEVICE_TABLE(of, msdc_of_ids); > > > > @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct > > > > mmc_host *mmc, struct mmc_ios *ios) > > > > struct msdc_host *host = mmc_priv(mmc); > > > > int ret; > > > > > > > > + /* Skip setting supply if not supported */ > > > > + if (!mmc->supply.vqmmc) > > > > + return 0; > > > > + > > > > > > Hi Christian, > > > > > > I think here is no need. If you have not 'vqmmc' in the > > > dts, IS_ERR(mmc->supply.vqmmc) would be -ENODEV and the > > > corresponding > > > flow would not be executed. > > > > > > And another question, host->pins_default is just selected here, > > > that > > > would be lost. > > > > > > > if (!IS_ERR(mmc->supply.vqmmc)) { > > > > if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && > > > > ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > > > > @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct > > > > mmc_host *mmc, int enb) > > > > dev_dbg(host->dev, "SDIO eint irq: > > > > %d!\n", host->eint_irq); > > > > } > > > > > > > > - pinctrl_select_state(host->pinctrl, host- > > > > > pins_uhs); > > > > > > > > + /* Skip setting uhs pins if not supported */ > > > > + if (host->pins_uhs) > > > > + pinctrl_select_state(host->pinctrl, > > > > host->pins_uhs); > > > > } else { > > > > dev_pm_clear_wake_irq(host->dev); > > > > } > > > > @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct > > > > mmc_host > > > > *mmc, struct mmc_ios *ios) > > > > > > > > msdc_set_buswidth(host, ios->bus_width); > > > > > > > > + /* Skip regulator if not supported */ > > > > + if (!mmc->supply.vmmc) > > > > + goto skip_regulator; > > > > + > > > > > > No need too. > > > > > > > /* Suspend/Resume will do power off/on */ > > > > switch (ios->power_mode) { > > > > case MMC_POWER_UP: > > > > @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct > > > > mmc_host > > > > *mmc, struct mmc_ios *ios) > > > > break; > > > > } > > > > > > > > +skip_regulator: > > > > if (host->mclk != ios->clock || host->timing != ios->timing) > > > > msdc_set_mclk(host, ios->timing, ios->clock); > > > > } > > > > @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct > > > > platform_device *pdev, > > > > if (IS_ERR(host->src_clk)) > > > > return PTR_ERR(host->src_clk); > > > > > > > > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > > > - if (IS_ERR(host->h_clk)) > > > > - return PTR_ERR(host->h_clk); > > > > + /* AN7581 SoC doesn't have hclk */ > > > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > > > + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > > > > + if (IS_ERR(host->h_clk)) > > > > + return PTR_ERR(host->h_clk); > > > > + } > > > > > > devm_clk_get_optional could be used to instead here, no need to use > > > compatible to distinguish. > > > > > > > I can make the hclk optional but I think this would affect also every > > other compatible by hiding broken clock configuration. > > > > > > host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > > > > if (IS_ERR(host->bus_clk)) > > > > @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct > > > > platform_device *pdev) > > > > return PTR_ERR(host->pins_default); > > > > } > > > > > > > > - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > > > "state_uhs"); > > > > - if (IS_ERR(host->pins_uhs)) { > > > > - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); > > > > - return PTR_ERR(host->pins_uhs); > > > > + /* AN7581 doesn't have state_uhs pins */ > > > > + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { > > > > + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, > > > > "state_uhs"); > > > > + if (IS_ERR(host->pins_uhs)) { > > > > + dev_err(&pdev->dev, "Cannot find pinctrl > > > > uhs!\n"); > > > > + return PTR_ERR(host->pins_uhs); > > > > + } > > > > } > > > > > > Could you consider to set a dummy 'state_uhs' same as > > > 'state_default' > > > in the dts, that you could not use compatible to distinguish here. > > > > > > > This is problematic, correct me if I'm wrong, you are suggesting to > > assign the emmc pins to both default and uhs? This is problematic as > > the > > pinctrl driver would complain that such pins are already assigned to > > something. Also I don't think it's possible to assign these pins to a > > dummy pin. > > > Maybe I have not expressed clearly...What I mean is that you could set > as below, and the content of &mmc_pins_uhs is just copied from > &mmc_pins_default. > > mmc@1fa0e000 { > ... > pinctrl-names = "default", "state_uhs"; > pinctrl-0 = <&mmc_pins_default>; > pinctrl-1 = <&mmc_pins_uhs>; > } Ok my bad. I did declared the second pin to pinctrl-0 instead of adding pinctrl-1. With that it does work correctly. > > > > > > > > /* Support for SDIO eint irq ? */ > > > > @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct > > > > platform_device *pdev) > > > > dev_err(&pdev->dev, "Cannot ungate clocks!\n"); > > > > goto release_clk; > > > > } > > > > + > > > > + /* AN7581 without regulator require tune to OCR values */ > > > > + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && > > > > + !mmc->ocr_avail) > > > > + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; > > > > + > > > > > > Maybe you could use regulator-fixed in the dts and configure > > > min/max > > > voltage to get ocr_avail, no need to set hard code here. > > > > > > > Also suggested by Wenbin Mei (梅文彬) and I just tried this. > > > > This can't be done, fixed-regulator needs to have the same min and > > max > > voltage or they fail to probe sooo fixed-regulator saddly can't be > > used > > :( > > > > I will send a new version of this with the other point corrected but > > I > > think a compatible and these additional if is a must :( > If use the fixed regulator such as below, you will get the same > ocr_avail as 'MMC_VDD_32_33 | MMC_VDD_33_34' through > mmc_regulator_get_ocrmask(). > > vmmc_3v3: regulator-vmmc-3v3 { > compatible = "regulator-fixed"; > regulator-name = "vmmc"; > regulator-min-microvolt = <3300000>; > regulator-max-microvolt = <3300000>; > regulator-always-on; > } Ok the code was a bit confusing but yes I can confirm that a 3.3 fixed regulator define those 2 flags so also this is OK. There is still the discussion about clock. You are totally against a new compatible for the hclk? > > > > > > msdc_init_hw(host); > > > > > > > > if (mmc->caps2 & MMC_CAP2_CQE) { > > > > -- > > Ansuel
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c index efb0d2d5716b..9d6868883c91 100644 --- a/drivers/mmc/host/mtk-sd.c +++ b/drivers/mmc/host/mtk-sd.c @@ -666,6 +666,20 @@ static const struct mtk_mmc_compatible mt8196_compat = { .support_new_rx = true, }; +static const struct mtk_mmc_compatible an7581_compat = { + .clk_div_bits = 12, + .recheck_sdio_irq = true, + .hs400_tune = false, + .pad_tune_reg = MSDC_PAD_TUNE0, + .async_fifo = true, + .data_tune = true, + .busy_check = true, + .stop_clk_fix = true, + .stop_dly_sel = 3, + .enhance_rx = true, + .support_64g = false, +}; + static const struct of_device_id msdc_of_ids[] = { { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, @@ -680,7 +694,7 @@ static const struct of_device_id msdc_of_ids[] = { { .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat}, { .compatible = "mediatek,mt8196-mmc", .data = &mt8196_compat}, { .compatible = "mediatek,mt8516-mmc", .data = &mt8516_compat}, - + { .compatible = "airoha,an7581-mmc", .data = &an7581_compat}, {} }; MODULE_DEVICE_TABLE(of, msdc_of_ids); @@ -1600,6 +1614,10 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios) struct msdc_host *host = mmc_priv(mmc); int ret; + /* Skip setting supply if not supported */ + if (!mmc->supply.vqmmc) + return 0; + if (!IS_ERR(mmc->supply.vqmmc)) { if (ios->signal_voltage != MMC_SIGNAL_VOLTAGE_330 && ios->signal_voltage != MMC_SIGNAL_VOLTAGE_180) { @@ -1699,7 +1717,9 @@ static void msdc_enable_sdio_irq(struct mmc_host *mmc, int enb) dev_dbg(host->dev, "SDIO eint irq: %d!\n", host->eint_irq); } - pinctrl_select_state(host->pinctrl, host->pins_uhs); + /* Skip setting uhs pins if not supported */ + if (host->pins_uhs) + pinctrl_select_state(host->pinctrl, host->pins_uhs); } else { dev_pm_clear_wake_irq(host->dev); } @@ -2036,6 +2056,10 @@ static void msdc_ops_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) msdc_set_buswidth(host, ios->bus_width); + /* Skip regulator if not supported */ + if (!mmc->supply.vmmc) + goto skip_regulator; + /* Suspend/Resume will do power off/on */ switch (ios->power_mode) { case MMC_POWER_UP: @@ -2071,6 +2095,7 @@ static void msdc_ops_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) break; } +skip_regulator: if (host->mclk != ios->clock || host->timing != ios->timing) msdc_set_mclk(host, ios->timing, ios->clock); } @@ -2816,9 +2841,12 @@ static int msdc_of_clock_parse(struct platform_device *pdev, if (IS_ERR(host->src_clk)) return PTR_ERR(host->src_clk); - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); - if (IS_ERR(host->h_clk)) - return PTR_ERR(host->h_clk); + /* AN7581 SoC doesn't have hclk */ + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { + host->h_clk = devm_clk_get(&pdev->dev, "hclk"); + if (IS_ERR(host->h_clk)) + return PTR_ERR(host->h_clk); + } host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); if (IS_ERR(host->bus_clk)) @@ -2926,10 +2954,13 @@ static int msdc_drv_probe(struct platform_device *pdev) return PTR_ERR(host->pins_default); } - host->pins_uhs = pinctrl_lookup_state(host->pinctrl, "state_uhs"); - if (IS_ERR(host->pins_uhs)) { - dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); - return PTR_ERR(host->pins_uhs); + /* AN7581 doesn't have state_uhs pins */ + if (!device_is_compatible(&pdev->dev, "airoha,an7581-mmc")) { + host->pins_uhs = pinctrl_lookup_state(host->pinctrl, "state_uhs"); + if (IS_ERR(host->pins_uhs)) { + dev_err(&pdev->dev, "Cannot find pinctrl uhs!\n"); + return PTR_ERR(host->pins_uhs); + } } /* Support for SDIO eint irq ? */ @@ -3010,6 +3041,12 @@ static int msdc_drv_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Cannot ungate clocks!\n"); goto release_clk; } + + /* AN7581 without regulator require tune to OCR values */ + if (device_is_compatible(&pdev->dev, "airoha,an7581-mmc") && + !mmc->ocr_avail) + mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; + msdc_init_hw(host); if (mmc->caps2 & MMC_CAP2_CQE) {
Add support for AN7581 MMC Host. The MMC Host controller is based on mt7622 with the difference of not having regulator supply and state_uhs pins and hclk clock. Some minor fixes are applied to check if the state_uhs pins are defined and make hclk optional for the new airoha compatible. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/mmc/host/mtk-sd.c | 55 ++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 9 deletions(-)