Messages in this thread | | | Subject | Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC | From | Michal Simek <> | Date | Wed, 7 Oct 2020 11:10:42 +0200 |
| |
On 07. 10. 20 10:55, Andy Shevchenko wrote: > On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > > ... > >>> + /* >>> + * This is like final gatekeeper. Need to ensure changed voltage > > like a final > >>> + * is settled before and after turn on this bit. >>> + */ > > ... > >>> + /* >>> + * This is like final gatekeeper. Need to ensure changed voltage > > Likewise. > >>> + * is settled before and after turn on this bit. >>> + */ > > ... > >>> + struct device *dev = &pdev->dev; >> >> nit: I got this but as I see 3 lines below maybe would be better to use >> it everywhere but it can be done in separate patch. > > In that case I think it would be better to have that patch first. It > make follow up code cleaner. > > ... > >>> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >>> + struct gpio_desc *uhs; >>> + >>> + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); >> >> I can't see change in dt binding to record uhs gpio. >> >> >> Better >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >> GPIOD_OUT_HIGH); >> >> then you can avoid uhs variable. > > Actually it's readability vs. additional variable. It was my > suggestion to have a variable to make readability better. > Are you insisting on this change?
I understand that it is just about preference. I would use it directly not to deal with it. If your preference is via variable I am fine with it too.
Thanks, Michal
| |