lkml.org 
[lkml]   [2022]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] gpiolib: of: Use correct fwnode for DT-probed chips
    On Wed, Nov 16, 2022 at 12:38:24PM +0200, Andy Shevchenko wrote:
    > On Wed, Nov 16, 2022 at 11:26:34AM +0100, Thierry Reding wrote:
    > > On Tue, Nov 15, 2022 at 12:18:00PM +0100, Marijn Suijten wrote:
    > > > On 2022-11-14 16:15:25, Brian Masney wrote:
    > > > > On Fri, Nov 11, 2022 at 12:37:32PM +0100, Thierry Reding wrote:
    > > > > > From: Thierry Reding <treding@nvidia.com>
    > > > > >
    > > > > > The OF node store in chip->fwnode is used to explicitly override the FW
    > > > > > node for a GPIO chip. For chips that use the default FW node (i.e. that
    > > > > > of their parent device), this will be NULL and cause the chip not to be
    > > > > > fully registered.
    > > > > >
    > > > > > Instead, use the GPIO device's FW node, which is set to either the node
    > > > > > of the parent device or the explicit override in chip->fwnode.
    > > > > >
    > > > > > Fixes: 8afe82550240 ("gpiolib: of: Prepare of_gpiochip_add() / of_gpiochip_remove() for fwnode")
    > > > > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
    > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
    > > > >
    > > > > Reviewed-by: Brian Masney <bmasney@redhat.com>
    > > > > Tested-by: Brian Masney <bmasney@redhat.com>
    > > > >
    > > > > I separately sent a similar type of patch to fix the same issue today:
    > > > > https://lore.kernel.org/linux-arm-msm/20221114202943.2389489-1-bmasney@redhat.com/T/#u
    > > >
    > > > For completeness, your linked patch fixes a synchronous external abort
    > > > on multiple Qualcomm platforms pointed out in [1]. This patch however
    > > > does not, are you sure they fix the exact same issue?
    >
    > Yes, they fix the same issue.
    >
    > > > [1]: https://lore.kernel.org/linux-arm-msm/20221115110800.35gl3j43lmbxm3jb@SoMainline.org/
    > >
    > > Can you check if the below fixes the MSM issue that you're seeing
    > > (applied on top of my earlier patch, though with Brian's reverted
    > > temporarily)?
    >
    > I don't know why we would need this. Brian's patch (already applied into
    > GPIO tree) is correct, no? (Moreover, it makes yours unneeded, but I'm fine
    > with having it anyway.)

    I've explained this in the reply to Brian's patch, but I don't think we
    want to use gc->fwnode other than initially to override the fwnode that
    the GPIO chip uses. It might even be worth turning it into a parameter
    to gpiochip_add() to avoid the ambiguity we have right now where we
    store the same fwnode in two different places and end up using either
    depending on whoever wrote the patch and what mood they were in. There
    really should only be one place to store this pointer to avoid this sort
    of ambiguity.

    Thierry
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2022-11-16 15:08    [W:4.474 / U:0.968 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site