Hi Krzysztof, >> Add Realtek mmc driver to make good use Synopsys DesignWare mmc cmdq >> host driver. >> >> Signed-off-by: Jyan Chou <jyanchou@xxxxxxxxxxx> >> >> --- >> v4 -> v5: >> - Fix linux coding style issues. >> - Modify the use of sizeof(*). >> - Remove useless function and parameter passing. >> - Replace platform_get_resource by devm_platform_ioremap_resource(). > It's merge window. Sending big patchset every day won't get you far. Thanks for your remind. >> >> v3 -> v4: >> - Modify dma setting's code to fix linux coding style. >> - Drop useless function messages. >> - Remove MODULE_ALIAS(). >> >> v0 -> v1: >> - Fix the compiler complains. >> --- >> + >> +static int dw_mci_rtk_parse_dt(struct dw_mci *host) { >> + struct dw_mci_rtkemmc_host *priv; >> + const u32 *prop; >> + int size, ret; >> + >> + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + priv->pinctrl = devm_pinctrl_get(host->dev); >> + if (IS_ERR(priv->pinctrl)) >> + dev_dbg(host->dev, "no pinctrl\n"); >> + >> + priv->pins_default = pinctrl_lookup_state(priv->pinctrl, >> + PINCTRL_STATE_DEFAULT); >> + if (IS_ERR(priv->pins_default)) >> + dev_warn(host->dev, "could not get default state\n"); >> + >> + priv->pins_sdr50 = pinctrl_lookup_state(priv->pinctrl, >> + "sdr50"); >> + if (IS_ERR(priv->pins_sdr50)) >> + dev_warn(host->dev, "could not get sdr50 state\n"); >> + >> + priv->pins_hs200 = pinctrl_lookup_state(priv->pinctrl, >> + "hs200"); >> + if (IS_ERR(priv->pins_hs200)) >> + dev_warn(host->dev, "could not get hs200 state\n"); >> + >> + priv->pins_hs400 = pinctrl_lookup_state(priv->pinctrl, >> + "hs400"); >> + if (IS_ERR(priv->pins_hs400)) >> + dev_warn(host->dev, "could not get hs400 state\n"); >> + >> + priv->pins_tune0 = pinctrl_lookup_state(priv->pinctrl, >> + "tune0"); >> + if (IS_ERR(priv->pins_tune0)) >> + dev_warn(host->dev, "could not get tune0 state\n"); >> + >> + priv->pins_tune1 = pinctrl_lookup_state(priv->pinctrl, >> + "tune1"); >> + if (IS_ERR(priv->pins_tune1)) >> + dev_warn(host->dev, "could not get tune1 state\n"); >> + >> + priv->pins_tune2 = pinctrl_lookup_state(priv->pinctrl, >> + "tune2"); >> + if (IS_ERR(priv->pins_tune2)) >> + dev_warn(host->dev, "could not get tune2 state\n"); >> + >> + priv->pins_tune3 = pinctrl_lookup_state(priv->pinctrl, >> + "tune3"); >> + if (IS_ERR(priv->pins_tune3)) >> + dev_warn(host->dev, "could not get tune3 state\n"); >> + >> + priv->pins_tune4 = pinctrl_lookup_state(priv->pinctrl, >> + "tune4"); >> + >> + if (IS_ERR(priv->pins_tune4)) >> + dev_warn(host->dev, "could not get tune4 state\n"); >> + >> + priv->vp0 = devm_clk_get(host->dev, "vp0"); >> + if (IS_ERR(priv->vp0)) >> + dev_err_probe(host->dev, ret, "could not get vp0 >> + clk\n"); >> + >> + priv->vp1 = devm_clk_get(host->dev, "vp1"); >> + if (IS_ERR(priv->vp1)) >> + dev_err_probe(host->dev, ret, "could not get vp1 >> + clk\n"); >> + >> + if (of_property_read_bool(host->dev->of_node, "supports-cqe")) >> + priv->is_cqe = 1; >> + else >> + priv->is_cqe = 0; >> + >> + prop = of_get_property(host->dev->of_node, "rdq-ctrl", &size); > NAK I will remove useless bindings. >> + if (prop) >> + priv->rdq_ctrl = of_read_number(prop, 1); >> + else >> + priv->rdq_ctrl = 0; >> + >> + priv->emmc_mode = 3; >> + >> + priv->m2tmx = >> + syscon_regmap_lookup_by_phandle(host->dev->of_node, >> + "realtek,m2tmx"); > NAK, for the same reasons I mentioned for other patch. > I will keep NAK-ing till you start testing your DTS. Sorry, I am not sure whether I modify the correct thing you said. I will add realtek,m2tmx in bindings. >> + if (IS_ERR_OR_NULL(priv->m2tmx)) >> + dev_err_probe(host->dev, ret, "can not get m2mtx >> + node.\n"); >> + >> + host->priv = priv; >> + >> + return 0; >> +} -----Original Message----- From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> Sent: Thursday, November 2, 2023 4:57 PM To: Jyan Chou [周芷安] <jyanchou@xxxxxxxxxxx>; ulf.hansson@xxxxxxxxxx; adrian.hunter@xxxxxxxxx; jh80.chung@xxxxxxxxxxx; riteshh@xxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; asutoshd@xxxxxxxxxxxxxx Cc: p.zabel@xxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; arnd@xxxxxxxx; briannorris@xxxxxxxxxxxx; doug@xxxxxxxxxxxxx; tonyhuang.sunplus@xxxxxxxxx; abel.vesa@xxxxxxxxxx; william.qiu@xxxxxxxxxxxxxxxx Subject: Re: [PATCH V5][3/4] mmc: Add dw mobile mmc cmdq rtk driver External mail. On 02/11/2023 09:15, Jyan Chou wrote: > Add Realtek mmc driver to make good use Synopsys DesignWare mmc cmdq > host driver. > > Signed-off-by: Jyan Chou <jyanchou@xxxxxxxxxxx> > > --- > v4 -> v5: > - Fix linux coding style issues. > - Modify the use of sizeof(*). > - Remove useless function and parameter passing. > - Replace platform_get_resource by devm_platform_ioremap_resource(). It's merge window. Sending big patchset every day won't get you far. > > v3 -> v4: > - Modify dma setting's code to fix linux coding style. > - Drop useless function messages. > - Remove MODULE_ALIAS(). > > v0 -> v1: > - Seperate different support into single patch. > - Fix the compiler complains. > --- > + > +static int dw_mci_rtk_parse_dt(struct dw_mci *host) { > + struct dw_mci_rtkemmc_host *priv; > + const u32 *prop; > + int size, ret; > + > + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->pinctrl = devm_pinctrl_get(host->dev); > + if (IS_ERR(priv->pinctrl)) > + dev_dbg(host->dev, "no pinctrl\n"); > + > + priv->pins_default = pinctrl_lookup_state(priv->pinctrl, > + PINCTRL_STATE_DEFAULT); > + if (IS_ERR(priv->pins_default)) > + dev_warn(host->dev, "could not get default state\n"); > + > + priv->pins_sdr50 = pinctrl_lookup_state(priv->pinctrl, > + "sdr50"); > + if (IS_ERR(priv->pins_sdr50)) > + dev_warn(host->dev, "could not get sdr50 state\n"); > + > + priv->pins_hs200 = pinctrl_lookup_state(priv->pinctrl, > + "hs200"); > + if (IS_ERR(priv->pins_hs200)) > + dev_warn(host->dev, "could not get hs200 state\n"); > + > + priv->pins_hs400 = pinctrl_lookup_state(priv->pinctrl, > + "hs400"); > + if (IS_ERR(priv->pins_hs400)) > + dev_warn(host->dev, "could not get hs400 state\n"); > + > + priv->pins_tune0 = pinctrl_lookup_state(priv->pinctrl, > + "tune0"); > + if (IS_ERR(priv->pins_tune0)) > + dev_warn(host->dev, "could not get tune0 state\n"); > + > + priv->pins_tune1 = pinctrl_lookup_state(priv->pinctrl, > + "tune1"); > + if (IS_ERR(priv->pins_tune1)) > + dev_warn(host->dev, "could not get tune1 state\n"); > + > + priv->pins_tune2 = pinctrl_lookup_state(priv->pinctrl, > + "tune2"); > + if (IS_ERR(priv->pins_tune2)) > + dev_warn(host->dev, "could not get tune2 state\n"); > + > + priv->pins_tune3 = pinctrl_lookup_state(priv->pinctrl, > + "tune3"); > + if (IS_ERR(priv->pins_tune3)) > + dev_warn(host->dev, "could not get tune3 state\n"); > + > + priv->pins_tune4 = pinctrl_lookup_state(priv->pinctrl, > + "tune4"); > + > + if (IS_ERR(priv->pins_tune4)) > + dev_warn(host->dev, "could not get tune4 state\n"); > + > + priv->vp0 = devm_clk_get(host->dev, "vp0"); > + if (IS_ERR(priv->vp0)) > + dev_err_probe(host->dev, ret, "could not get vp0 > + clk\n"); > + > + priv->vp1 = devm_clk_get(host->dev, "vp1"); > + if (IS_ERR(priv->vp1)) > + dev_err_probe(host->dev, ret, "could not get vp1 > + clk\n"); > + > + if (of_property_read_bool(host->dev->of_node, "supports-cqe")) > + priv->is_cqe = 1; > + else > + priv->is_cqe = 0; > + > + prop = of_get_property(host->dev->of_node, "rdq-ctrl", &size); NAK > + if (prop) > + priv->rdq_ctrl = of_read_number(prop, 1); > + else > + priv->rdq_ctrl = 0; > + > + priv->emmc_mode = 3; > + > + priv->m2tmx = > + syscon_regmap_lookup_by_phandle(host->dev->of_node, > + "realtek,m2tmx"); NAK, for the same reasons I mentioned for other patch. I will keep NAK-ing till you start testing your DTS. > + if (IS_ERR_OR_NULL(priv->m2tmx)) > + dev_err_probe(host->dev, ret, "can not get m2mtx > + node.\n"); > + > + host->priv = priv; > + > + return 0; > +} Best regards, Krzysztof