RE: [PATCH V5][3/4] mmc: Add dw mobile mmc cmdq rtk driver

From: Jyan Chou [周芷安]
Date: Thu Nov 09 2023 - 02:36:30 EST

Next message: Jayant Chowdhary: "Re: [PATCH v5] usb:gadget:uvc Do not use worker thread to pump isoc usb requests"
Previous message: kernel test robot: "drivers/hwtracing/intel_th/msu.c:131: warning: Function parameter or member 'msu_base' not described in 'msc'"
Next in thread: Krzysztof Kozlowski: "Re: [PATCH V5][3/4] mmc: Add dw mobile mmc cmdq rtk driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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

Next message: Jayant Chowdhary: "Re: [PATCH v5] usb:gadget:uvc Do not use worker thread to pump isoc usb requests"
Previous message: kernel test robot: "drivers/hwtracing/intel_th/msu.c:131: warning: Function parameter or member 'msu_base' not described in 'msc'"
Next in thread: Krzysztof Kozlowski: "Re: [PATCH V5][3/4] mmc: Add dw mobile mmc cmdq rtk driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]