Received: by 2002:a05:6a10:5bc5:0:0:0:0 with SMTP id os5csp364812pxb; Wed, 3 Nov 2021 05:37:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwi+t/BNN8PiTfrWpK0EaBMDk6CyUJN8LaGzPK8UUmslqzPLP1a4f/M5HrPpaX5W5vUCkNM X-Received: by 2002:a02:ce8c:: with SMTP id y12mr21195372jaq.116.1635943025126; Wed, 03 Nov 2021 05:37:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1635943025; cv=none; d=google.com; s=arc-20160816; b=mfZ2wqfku2lm3n8g1C12xYeC7J3vJfzU4K8Bfk9D6BC6k/bTnO6f16gMHGlRLVfgBD Tb5GuUlD+lP/kxwwgfYr7VrJVSQpzAzqktA10+q5p1s5UmwFGASfNDxX7UGMI2qEJdO6 OKX4eNaeBtYI7psc4W04a/Mr5gb4TnpeoSZjVa0JEYFqc7eJ68Ji3xL4hd2NBh+gmEmE LZLVN19GExudSpeHydW1yFl/uHRAHJ/kGfTq3pRF6Gnu+bSsTWHTTWV4oK5zz3qIEGsQ fzSySPBp9jiVuuJF9vKn5yxFlSlAyibMHRZQ4NZWsXurmgK040lRpOvz5tXp27Hn4M+T LfYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version; bh=JE/2edLy6usLclELy9HdQo+SFIip3f5Fv/danfH38Us=; b=E1mcX1Zvpchk9qCzgGFqF/Dt5bo/VmQqSxOZRlhhxFu/yP8AOdA8NcKoeYQPG47lH7 gIEV47pk7BBOxqVR18+3eihceIeXS0LGoGLw1GeyhPC7/bNAOdeRhcTByM9hOhf+6fDV VSZBxU7Cre8tcFTwvD0fUlpg+UdJ/gr+toyWzIe0vMC6KbHbVVPrBMHobvpm6PeW7rm2 z7qswEaG/2W+D9pQyFsRbRCoug5UJhwmlwRhuRrmgqVgVyq2L4fysCsGCWsE9C5HPP04 aUZrjU2VDKEkE7FO6fL7fA4nPtEDl8DTP1th4Syv4dNIQh3K0lt1rE8hgWzDN2tk5rFx EjuQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id u11si2734843ilg.140.2021.11.03.05.36.51; Wed, 03 Nov 2021 05:37:05 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232012AbhKCMi3 (ORCPT + 99 others); Wed, 3 Nov 2021 08:38:29 -0400 Received: from mail-pf1-f171.google.com ([209.85.210.171]:39933 "EHLO mail-pf1-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231993AbhKCMiL (ORCPT ); Wed, 3 Nov 2021 08:38:11 -0400 Received: by mail-pf1-f171.google.com with SMTP id x64so2161555pfd.6; Wed, 03 Nov 2021 05:35:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=JE/2edLy6usLclELy9HdQo+SFIip3f5Fv/danfH38Us=; b=MqdddXuI5fFJnjftC8vLsoy2ixpIPYuFiM8V4oraxQUT2habCa1/OyVh9Or5gfhlsh 54/3R9jpxylyL59NOJYqx8AUw6mSbJYhw3biuhKm2adWmuVZACknH8ZNuHnUZ/0uG02b OmCWlvN9TX3iMY7IGv/WKbv/v474vSrqB5kS2c3wn0ULhRtAyJOknq+t95nW0Lk3Zy95 LwBc1lj4j6oz9UtSbAf4l+osDv3bZAC0xQrNf+PSeygV8MS7manzlCFKNnK9bygpN0nV urkvQwnd3LfAxkZ1ViFujKVTKGi7EUHwsYYGz9KDcDPuWp7eSZpDHhlHPFqipgi5r+y/ CwVA== X-Gm-Message-State: AOAM5311AHT7zBa2IzVqTifPUN7BErbjkrw5HebaKSMTVY+NoFg2980c p0zTd45IlXYqMXRDyQCC7ZeMNrxMoUunQXhJGPU= X-Received: by 2002:a63:2d46:: with SMTP id t67mr32735713pgt.15.1635942934696; Wed, 03 Nov 2021 05:35:34 -0700 (PDT) MIME-Version: 1.0 References: <20211102161125.1144023-1-kernel@esmil.dk> <20211102161125.1144023-13-kernel@esmil.dk> In-Reply-To: From: Emil Renner Berthing Date: Wed, 3 Nov 2021 13:35:23 +0100 Message-ID: Subject: Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs To: Andy Shevchenko Cc: linux-riscv , devicetree , linux-clk , "open list:GPIO SUBSYSTEM" , "open list:SERIAL DRIVERS" , Palmer Dabbelt , Paul Walmsley , Rob Herring , Michael Turquette , Stephen Boyd , Thomas Gleixner , Marc Zyngier , Philipp Zabel , Linus Walleij , Greg Kroah-Hartman , Daniel Lezcano , Andy Shevchenko , Jiri Slaby , Maximilian Luz , Sagar Kadam , Drew Fustini , Geert Uytterhoeven , Michael Zhu , Fu Wei , Anup Patel , Atish Patra , Matteo Croce , Linux Kernel Mailing List , Huan Feng Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 3 Nov 2021 at 10:13, Andy Shevchenko wrote: > On Tue, Nov 2, 2021 at 10:35 PM Emil Renner Berthing wrote: > > On Tue, 2 Nov 2021 at 21:02, Andy Shevchenko wrote: > > > On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing wrote: > > > > > + switch (trigger) { > > > > > + default: > > > > > > > + irq_set_handler_locked(d, handle_bad_irq); > > > > > > Why? You have it already in ->probe(), what's the point? > > > > So last time you asked about this, I explained a situation where > > userspace first grabs a GPIO, set the interrupt to edge triggered, and > > then later loads a driver that requests an unsupported IRQ type. > > I didn't get this scenario. Is it real? No, it's totally made up, but I mean we even have tools like fuzzing to help us find bugs that would never happen in real use cases. > > Then > > I'd like to set the handler back to handle_bad_irq so we don't get > > weird interrupts, but maybe now you know a reason why that doesn't > > matter or can't happen? > > In ->probe() you set _default_ handler to bad(), what do you mean by > 'set the handler back to bad()'? How is it otherwise if you free an > interrupt? It might not be, but when not sure I thought it better to error on the safe side. > So, please elaborate with call traces what the scenario / use case you > are talking about. If it's true what you are saying, we have a > situation (plenty of GPIO drivers don't do what you are suggesting > here). > > > > > + return -EINVAL; > > > > + } > > ... > > > > > + ret = reset_control_deassert(rst); > > > > + if (ret) > > > > + return dev_err_probe(dev, ret, "could not deassert resetd\n"); > > > > > > > + ret = devm_pinctrl_register_and_init(dev, &starfive_desc, sfp, &sfp->pctl); > > > > + if (ret) > > > > > > I don't see who will assert reset here. > > > > No, so originally this driver would first assert and then deassert > > reset. I decided against that because in all likelyhood earlier boot > > stages would have set pinmux up for a serial port, and we don't want > > to interrupt the serial debug output. The only reason I make sure the > > reset line is deasserted is in case someone makes a really minimal > > bootloader that just does the absolute minimal to load a Linux kernel > > and doesn't even log any anything. > > > > By the same token we also don't want to assert reset on error in case > > it resets pin muxing for the the serial line that was supposed to log > > the error. > > Perhaps comment in the code explaining this? Sure.