Re: [PATCH v7][2/4] mmc: Add Synopsys DesignWare mmc cmdq host driver

["Arnd Bergmann" <arnd@arndb.de>]

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