On Mon 09 Oct 17:17 PDT 2017, Fenglin Wu wrote: > On 10/9/2017 1:56 PM, Bjorn Andersson wrote: > > On Sun 08 Oct 22:34 PDT 2017, Fenglin Wu wrote: > > > > > On 10/6/2017 12:27 AM, Bjorn Andersson wrote: [..] > > > > But I spotted another issue while reviewing this; currently the initial > > > > state of is_enabled is unconditionally set to enabled in > > > > pmic_gpio_populate(), so reading the initial pinconf or configuring a > > > > pinmux before setting a pinconf will operate on the potentially wrong > > > > information. > > > > > > > > So I think the initial value should be read out from REG_EN_CTL rather > > > > than being just "true". > > > > > > > > Can you please either submit another patch for this? > > > > > > Hmm, considering a GPIO which is disabled by default in hardware > > > setting, what's its expected state if we only define "function" for it? > > > I was thinking we need to enable it once it has any setting in pinmux or > > > pinconf. If you think that we need to keep its original state until we > > > set pinconf for it, yes, I can submit a change to address this. > > > > > > > Are there valid cases where only function should be selected and no > > other configuration is used? If so it makes sense to make > > pmic_gpio_set_mux() enable the block. > > > > > > Regardless of this, if there are disabled pins that are not mentioned in > > DT they will still appear as enabled in the debugfs interface; and this > > I consider an error worth fixing. > How about we do both: read the HW initial state in pmic_gpio_populate(), > and also enable the GPIO block in pmic_gpio_set_mux()? > That sounds good. Please do this as two separate patches, with the commit message clearly describing a case where the pinconf does not affect the function of the pin, so a pinmux is the only thing needed. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
