Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-1234-gac66594aae-fm-20231122.001-gac66594a
MIME-Version: 1.0
Message-Id: <cf23b3a9-54cb-4c48-923e-8a05a8d4bc27@app.fastmail.com>
In-Reply-To: <23011695aafca595c3c8722fda2a8e194c5318df.camel@pengutronix.de>
References: <20231121091101.5540-1-jyanchou@realtek.com>
 <20231121091101.5540-3-jyanchou@realtek.com>
 <23011695aafca595c3c8722fda2a8e194c5318df.camel@pengutronix.de>
Date:   Tue, 28 Nov 2023 11:53:13 +0100
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Philipp Zabel" <p.zabel@pengutronix.de>,
        "Jyan Chou" <jyanchou@realtek.com>,
        "Ulf Hansson" <ulf.hansson@linaro.org>,
        "Adrian Hunter" <adrian.hunter@intel.com>, jh80.chung@samsung.com,
        riteshh@codeaurora.org, "Rob Herring" <robh+dt@kernel.org>,
        krzysztof.kozlowski+dt@linaro.org
Cc:     "Conor Dooley" <conor+dt@kernel.org>, asutoshd@codeaurora.org,
        "linux-mmc @ vger . kernel . org" <linux-mmc@vger.kernel.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        "Brian Norris" <briannorris@chromium.org>,
        "Doug Brown" <doug@schmorgal.com>,
        "Tony Huang" <tonyhuang.sunplus@gmail.com>,
        "Abel Vesa" <abel.vesa@linaro.org>, william.qiu@starfivetech.com
Subject: Re: [PATCH v7][2/4] mmc: Add Synopsys DesignWare mmc cmdq host driver
Content-Type: text/plain
Precedence: bulk

On Mon, Nov 27, 2023, at 13:51, Philipp Zabel wrote:
> On Di, 2023-11-21 at 17:10 +0800, Jyan Chou wrote:
>> diff --git a/drivers/mmc/host/dw_mmc_cqe.c b/drivers/mmc/host/dw_mmc_cqe.c
>> new file mode 100644
>> index 000000000000..eb00d6a474b2
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc_cqe.c
>> @@ -0,0 +1,1467 @@
> [...]
>> +#ifdef CONFIG_OF
>> +static struct dw_mci_board *dw_mci_cqe_parse_dt(struct dw_mci *host)
>> +{
>> +	struct dw_mci_board *pdata;
>> +	struct device *dev = host->dev;
>> +	const struct dw_mci_drv_data *drv_data = host->drv_data;
>> +	int ret;
>> +
>> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +	if (!pdata)
>> +		return ERR_PTR(-ENOMEM);
>> +
>
> There is no reason to hide device tree parsing errors here, and I'd
> argue pdata should not be returned with rstc set to an error value.
> devm_reset_control_get_optional_exclusive() returns NULL if there are
> no errors and no reset is specified in the device tree.
>
> Then you can just use dev_err_probe() at the call site in
> dw_mci_cqe_probe().

I think ideally the dw_mci_board should be merged into the dw_mci
structure, avoiding the extra kzalloc() step. Having separate
structures here is likely an artifact from an old version of the
driver that predates the use of devicetree, but since everything
now uses DT, there is no point in the extra abstraction.

     Arnd