| Message ID | 1513797033-9494-1-git-send-email-timur@codeaurora.org |
|---|---|
| Headers | show |
| Series | pinctrl: qcom: add support for sparse GPIOs | expand |
Hi Timur, thank you for your perseverance. I am sorry that I am sometimes not fast to respond :( On Wed, Dec 20, 2017 at 8:10 PM, Timur Tabi <timur@codeaurora.org> wrote: > Patch 1 reverts an old patch that triggers a get_direction of every > pin upon init, without attempting to request the pins first. The > direction is already being queried when the pin is requested. > > Patch 2 adds support to pinctrl-msm for "unavailable" GPIOs. I have applied both of these to the pinctrl "devel" branch so we can see if all is fine. They have Stephen's ACK so I am happy with them, I am just still slightly worried about possible regressions because of patch 1. > Patch 3 extends that support to pinctrl-qdf2xxx. A recent ACPI change > on QDF2400 platforms blocks access to most pins, so the driver can only > register a subset. I see this one is still under discussion. If nothing drastic happens with patch 1/2 in linux-next it should be fine if you just resend this single patch in subsequent submissions. I think it may be worthwhile to keep Andrew Cooks in the loop for future submissions as he's trying to solve similar problems for AMD. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/21, Linus Walleij wrote: > Hi Timur, > > thank you for your perseverance. I am sorry that I am sometimes not > fast to respond :( > > On Wed, Dec 20, 2017 at 8:10 PM, Timur Tabi <timur@codeaurora.org> wrote: > > > Patch 1 reverts an old patch that triggers a get_direction of every > > pin upon init, without attempting to request the pins first. The > > direction is already being queried when the pin is requested. > > > > Patch 2 adds support to pinctrl-msm for "unavailable" GPIOs. > > I have applied both of these to the pinctrl "devel" branch so we > can see if all is fine. > > They have Stephen's ACK so I am happy with them, I am just > still slightly worried about possible regressions because of > patch 1. > > > Patch 3 extends that support to pinctrl-qdf2xxx. A recent ACPI change > > on QDF2400 platforms blocks access to most pins, so the driver can only > > register a subset. > > I see this one is still under discussion. > > If nothing drastic happens with patch 1/2 in linux-next > it should be fine if you just resend this single patch in subsequent > submissions. > If we go with my suggestion, patch 2 is not necessary and should be dropped. The different approaches come down to expressing which pins are available through the gpio valid mask, or through the npins field of the msm pinctrl driver. Also, my approach covers more than just GPIOs, it covers irqs and adjusts the pinctrl pin request function so that pinctrl can't request unavailable pins.
On Wed, Dec 27, 2017 at 3:01 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 12/21, Linus Walleij wrote: >> Hi Timur, >> >> thank you for your perseverance. I am sorry that I am sometimes not >> fast to respond :( >> >> On Wed, Dec 20, 2017 at 8:10 PM, Timur Tabi <timur@codeaurora.org> wrote: >> >> > Patch 1 reverts an old patch that triggers a get_direction of every >> > pin upon init, without attempting to request the pins first. The >> > direction is already being queried when the pin is requested. >> > >> > Patch 2 adds support to pinctrl-msm for "unavailable" GPIOs. >> >> I have applied both of these to the pinctrl "devel" branch so we >> can see if all is fine. >> >> They have Stephen's ACK so I am happy with them, I am just >> still slightly worried about possible regressions because of >> patch 1. >> >> > Patch 3 extends that support to pinctrl-qdf2xxx. A recent ACPI change >> > on QDF2400 platforms blocks access to most pins, so the driver can only >> > register a subset. >> >> I see this one is still under discussion. >> >> If nothing drastic happens with patch 1/2 in linux-next >> it should be fine if you just resend this single patch in subsequent >> submissions. >> > > If we go with my suggestion, patch 2 is not necessary and should > be dropped. OK I have reverted it. > The different approaches come down to expressing > which pins are available through the gpio valid mask, or through > the npins field of the msm pinctrl driver. Also, my approach > covers more than just GPIOs, it covers irqs and adjusts the > pinctrl pin request function so that pinctrl can't request > unavailable pins. I agree, this is better. Would even patch 1 be needed after this? Maybe I should revert that too. Leaving that code in has the upside of showing the actual initial directions of GPIO lines even if they have not been requested, in e.g. debugfs. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/28, Linus Walleij wrote: > On Wed, Dec 27, 2017 at 3:01 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > > The different approaches come down to expressing > > which pins are available through the gpio valid mask, or through > > the npins field of the msm pinctrl driver. Also, my approach > > covers more than just GPIOs, it covers irqs and adjusts the > > pinctrl pin request function so that pinctrl can't request > > unavailable pins. > > I agree, this is better. Thanks for the feedback. I'll update and resend my patch to the list. > > Would even patch 1 be needed after this? Maybe I should > revert that too. Leaving that code in has the upside of showing > the actual initial directions of GPIO lines even if they have > not been requested, in e.g. debugfs. > Patch 1 is still needed. Without that patch, we'll be poking each GPIO to figure out the direction at boot without checking any valid mask or calling the request APIs. I don't see the part in debugfs where we show the direction of a GPIO if it hasn't been requested. Don't we skip over the unrequested GPIOs because of this code in gpiolib_dbg_show()? if (!test_bit(FLAG_REQUESTED, &gdesc->flags)) { ... continue; }
On Thu, Dec 28, 2017 at 5:15 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > Patch 1 is still needed. Without that patch, we'll be poking each > GPIO to figure out the direction at boot without checking any > valid mask or calling the request APIs. Aha, I just thought you'd augment that code to take valid GPIOs into account. Anyways, I left the patch in, so no worries. > I don't see the part in > debugfs where we show the direction of a GPIO if it hasn't been > requested. Don't we skip over the unrequested GPIOs because of > this code in gpiolib_dbg_show()? > > if (!test_bit(FLAG_REQUESTED, &gdesc->flags)) { > ... > continue; > } Yes you're right. This is more for lsgpio and the chardev side of things for inspecting lines really. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html