On 10/02/19 6:49 PM, Boris Brezillon wrote:
> On Tue, 5 Feb 2019 11:43:46 +0530
> Vignesh R <vigneshr@xxxxxx> wrote:
>
>>>> static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>> {
>>>> - const struct spi_nor_hwcaps hwcaps = {
>>>> - .mask = SNOR_HWCAPS_READ |
>>>> - SNOR_HWCAPS_READ_FAST |
>>>> - SNOR_HWCAPS_READ_1_1_2 |
>>>> - SNOR_HWCAPS_READ_1_1_4 |
>>>> - SNOR_HWCAPS_PP,
>>>> - };
>>>> struct platform_device *pdev = cqspi->pdev;
>>>> struct device *dev = &pdev->dev;
>>>> + const struct cqspi_driver_platdata *ddata;
>>>> + struct spi_nor_hwcaps hwcaps;
>>>> struct cqspi_flash_pdata *f_pdata;
>>>> struct spi_nor *nor;
>>>> struct mtd_info *mtd;
>>>> unsigned int cs;
>>>> int i, ret;
>>>>
>>>> + ddata = of_device_get_match_data(dev);
>>>> + if (!ddata)
>>>> + hwcaps.mask = CQSPI_BASE_HWCAPS_MASK;
>>>
>>> Now that .data is set in all cqspi_dt_ids[], maybe it's better to print a
>>> message and return an error here. But I guess it's a matter of taste, so not a
>>> show stopper.
>>
>> Since, driver data is kernel internal field, I guess there is little
>> help in printing out the error to the user when its missing. I prefer to
>> keep this as is, as basic Quad mode is supported by all versions of the IP.
>
> Well, if all compat entries have an associated platdata instance we
> don't need to support the !ddata case, right? I think enforcing the
> presence of such a platdata is actually is a sane thing to do (which
> implies returning an error when ddata is NULL).
>
Ok, will respin with a dev_err() and return -EINVAL when ddata is NULL.
--
Regards
Vignesh
