Messages in this thread | | | Date | Thu, 21 Apr 2022 10:32:57 +0530 | Subject | Re: [PATCH V4] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC | From | "Sajida Bhanu (Temp)" <> |
| |
Hi,
Thanks for the review.
Please find the inline comments.
Thanks,
Sajida
On 4/19/2022 12:52 PM, Philipp Zabel wrote: > Hi Sajida, > > On Di, 2022-04-19 at 11:46 +0530, Sajida Bhanu (Temp) wrote: > [...] >>>> +static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host) >>>> +{ >>>> + struct reset_control *reset; >>>> + int ret = 0; >>> No need to initialize ret. >>> >>>> + >>>> + reset = reset_control_get_optional_exclusive(dev, NULL); >>>> + if (IS_ERR(reset)) >>>> + return dev_err_probe(dev, PTR_ERR(reset), >>>> + "unable to acquire core_reset\n"); >>>> + >>>> + if (!reset) >>>> + return ret; >> Here we are returning ret directly if reset is NULL , so ret >> initialization is required. > You are right. I would just "return 0;" here, but this is correct as > is. Ok >>>> + >>>> + ret = reset_control_assert(reset); >>>> + if (ret) >>>> + return dev_err_probe(dev, ret, "core_reset assert failed\n"); >>> Missing reset_control_put(reset) in the error path. >> Sure will add >>>> + >>>> + /* >>>> + * The hardware requirement for delay between assert/deassert >>>> + * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to >>>> + * ~125us (4/32768). To be on the safe side add 200us delay. >>>> + */ >>>> + usleep_range(200, 210); >>>> + >>>> + ret = reset_control_deassert(reset); >>>> + if (ret) >>>> + return dev_err_probe(dev, ret, "core_reset deassert failed\n"); >>> Same as above. Maybe make both ret = dev_err_probe() and goto ... >> In both cases error message is different so I think goto not good idea here. > You could goto after the error message. Either way is fine.
Sorry didn't get this ..can you please help
> > regards > Philipp
| |