From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2CB4AC07545 for ; Tue, 24 Oct 2023 07:13:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=WQa4Gds064gcirsfurjppQc0Rjh2IS+mMzx4F8qqdGc=; b=Xd12fJa/z3w+TR iAnsY5WFZuY5KLEjtwGR4yn7AnhL7onDZ9jhkisHd/xxdvSPSvBqttj9UTX4OALdB//sUqfg8eF0Z HvZsVNMMkY+GVExbqOhxN9F1Ebf+3QqCxoDIZWfEPkfD4kvs5qZkB9NL3cp7MeorvO4H2yOMNXPxq URUrBjXeVtUg4g/S2auPkH8VA3V8x8I1wUETKRfWk1/+d1lxldZu1Xaa4PIU0V65RUHLySxyqsfaA svTFFkb91oeofPRc3voislrgDpDdF9n3AaQfy9RMPfxIEEkD47BTcnNDdTZ0S5S0tHAzXa5J1vqZG 4Feg25RJovAwSqTtclJw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qvBaf-0093Ep-1P; Tue, 24 Oct 2023 07:12:37 +0000 Received: from mail-pf1-x430.google.com ([2607:f8b0:4864:20::430]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qvBad-0093DL-0N for linux-arm-kernel@lists.infradead.org; Tue, 24 Oct 2023 07:12:36 +0000 Received: by mail-pf1-x430.google.com with SMTP id d2e1a72fcca58-6bd20c30831so1109819b3a.1 for ; Tue, 24 Oct 2023 00:12:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1698131552; x=1698736352; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=amFp0BVeavttuQpXCOb46m8XpZ/bsIpXzEjd5+Ix8vo=; b=NssVZJMkyXph80r5vRKrHbPdr/iUJaJFk8VQnvoyPyy+rLhrMQ5WbsRToMbMBg+AWq t+KxrF0wobLs6vvBZCEVgNXSjVxBUFVZnh8EzqF3w0Yg6BJn07NBPCljy49oXeC4WF3i eIcNyDbu3GClMnfkBfoHjXwy27JtxQiTixrGbweYP3T1eebDxCNAlxBTAUxCRPfokjuS /Q1ps7c7etY9IucQlggGf0hicRFAT0UwejL3qiD2aZbwHaLbBVD93T5mPuB6BJTffjN/ oLf9P8vK0be6W/hhu2uZ8/kmFAS5AoHT+isc9Nex2BKqwpNoAE2OZvV6baHrZqbaD895 om7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698131552; x=1698736352; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=amFp0BVeavttuQpXCOb46m8XpZ/bsIpXzEjd5+Ix8vo=; b=NV/vjuE51szVA2NySYgY2nq1RrVx0UIS8EZ9t/1TlKnetNhsJuHQAPcaM3x8nr0g0w era5rRgq2v8iP+ZvtSzx3D9X3d6LZPQw+47AYUwnMgbBIo1vq8vmg1rOnBFpqFjbvwQc ZonTUuT1CxE/8tdJBQ7lI3N3CvhEO0kgXCKl3azF/DpwcrvmTbyxbUppKAvvCUvTStUS 4X1AVnaJIWh4Ss0y/c85tIXwT5Mavj22Gma3BiKTJJHWcltftYIMk03g7yHRbLHdz/sQ zuRBfRUm9TAHg7fWmitSnmvJDGRHuSFjoGPnzM/4CO/gsI+Zzs8jr3VvoG6L9RANXsig gLWA== X-Gm-Message-State: AOJu0Yx5KI8sME93vqyF1Z/9K+MWq8O02cd36oHD1nDsfmhrhWHcrOIe hE0Lad0bJOxphUGx9u79kk3Lkw== X-Google-Smtp-Source: AGHT+IF7QdjXPOxjXon+i2oKSwso+lRkUoW9ta6jKJKZ5Gj11CuV1qcL5hoMrDkcOVH4ezKfGuWAow== X-Received: by 2002:a05:6a20:2451:b0:171:737:df97 with SMTP id t17-20020a056a20245100b001710737df97mr14371285pzc.2.1698131552304; Tue, 24 Oct 2023 00:12:32 -0700 (PDT) Received: from octopus ([2400:4050:c3e1:100:7c15:610f:1205:f10c]) by smtp.gmail.com with ESMTPSA id ch20-20020a17090af41400b0026b3f76a063sm6535391pjb.44.2023.10.24.00.12.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Oct 2023 00:12:31 -0700 (PDT) Date: Tue, 24 Oct 2023 16:12:27 +0900 From: AKASHI Takahiro To: Linus Walleij Cc: Rob Herring , sudeep.holla@arm.com, cristian.marussi@arm.com, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, Oleksii_Moisieiev@epam.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org Subject: Re: [RFC v2 5/5] dt-bindings: gpio: Add bindings for pinctrl based generic gpio driver Message-ID: Mail-Followup-To: AKASHI Takahiro , Linus Walleij , Rob Herring , sudeep.holla@arm.com, cristian.marussi@arm.com, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, Oleksii_Moisieiev@epam.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org References: <20231005025843.508689-1-takahiro.akashi@linaro.org> <20231005025843.508689-6-takahiro.akashi@linaro.org> <20231006132346.GA3426353-robh@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231024_001235_160151_97454193 X-CRM114-Status: GOOD ( 52.47 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Linus, On Mon, Oct 23, 2023 at 10:12:21AM +0200, Linus Walleij wrote: > Hi Takashi, > > sorry for slow response :( Thank you for your feedback. > On Tue, Oct 17, 2023 at 4:32???AM AKASHI Takahiro > wrote: > > > > > > We can probably mandate that this has to be inside a pin controller > > > > > since it is a first. > > > > > > > > Yeah, my U-Boot implementation tentatively supports both (inside and > > > > outside pin controller). But it is not a user's choice, but we should > > > > decide which way to go. > > > > > > OK I have decided we are going to put it inside the pin control node, > > > as a subnode. (I don't expect anyone to object.) > > > > While I'm still thinking of how I can modify my current implementation > > to fit into 'inside' syntax, there are a couple of concerns: > > > > 1) invoke gpiochip_add_data() at probe function > > Probably we no longer need "compatible" property, > > The DT binding people made it clear to me that they really > like compatibles for this kind of stuff so we should probably > keep it. Okay, I will assume this in the following discussion. > > but instead we need to > > call gpiochip_add_data() explicitly in SCMI pin controller's probe > > as follows: > > > > scmi_pinctrl_probe() > > ... > > devm_pinctrl_register_and_init(dev, ..., pctrldev); > > pinctrl_enable(pctrldev); > > > > device_for_each_child_node(dev, fwnode) > > if (fwnode contains "gpio-controller") { > > /* what pin_control_gpio_probe() does */ > > gc->get_direction = ...; > > ... > > devm_gpiochip_data_add(dev, gc, ...); > > } > > I think it is better of the pin controller just parse and add any > subdevices (GPIO or other) using of_platform_default_populate() > (just grep for this function and you will see how many device > drivers use that). IICU, then, we will have to add a "compatible" to pinctrl node to make of_platform_default_populate() work as expected. That is: scmi { ... protocol@19 { compatible = "simple-bus"; // <- added reg = <0x19>; ... // a couple of pinconf nodes scmi_gpio { compatible = "pin-control-gpio"; ... } } } Is this what you meant? In this case, however, "protocol@19" has a mixture of sub-nodes, most are pinconf definitions which are the properties of the pin controller, while "scmi_gpio" is a separate device. The code will work, but is it sane from DT binding pov? > What is good with this approach is that if you place this call > last in the probe() we know the GPIO driver has all resources > it needs when it probes so it won't defer. > > > 2) gpio-by-pinctrl.c > > While this file is SCMI-independent now, due to a change at (1), > > it would be better to move the whole content inside SCMI pin controller > > driver (because there is no other user for now). > > That works, too. I have no strong opinion on this subject. I assumed that "compatible" had been removed here. If we retain "compatible" property, I don't care either way. > > 3) Then, pin-control-gpio.yaml may also be put into SCMI binding > > (i.e. firmware/arm,scmi.yaml). Can we leave the gpio binding outside? > > There is no clear pattern whether to put subdevice bindings into > the parent device binding or not. Maybe? A lot of MFD devices does > this for sure. The same as above. > > 4) phandle in "gpio-ranges" property > > (As you mentioned) > > The first element in a tuple of "gpio-ranges" is a phandle to a pin > > controller node. Now that the gpio node is a sub node of pin controller, > > the phandle is trivial. But there is no easier way to represent it > > than using an explicit label: > > (My U-Boot implementation does this.) > > > > scmi { > > ... > > scmi_pinctrl: protocol@19 { > > ... > > gpio { > > gpio-controller; > > ... > > gpio-ranges = <&scmi_pinctrl ... >; > > } > > } > > } > > > > I tried: > > gpio-ranges = <0 ...>; // dtc passed, but '0' might be illegal by spec. > > gpio-ranges = <(-1) ...>; // dtc passed, but ... > > gpio-ranges = <&{..} ...>; // dtc error because it's not a full path. > > > > Do you have any other idea? Otherwise, I will modify my RFC > > with the changes above. > > If you have the GPIO node inside the pin controller node > and have all the details of the existing ranges available, there > is no need to put that into the device tree at all, just omit it? Then, of_gpiochip_add_data() (hence, of_gpiochip_add()/gpiochip_add_data()) won't work because it always assume phandle + 3 arguments. Right? In this case, "gpio-ranges" here may have different semantics from the other pinctrl-based gpio drivers. Doesn't matter from DT pov? > Instead just call gpiochip_add_pin_range() directly in Linux > after adding the pin controller and gpio_chip. > C.f. drivers/pinctrl/pinctrl-sx150x.c for an example of a driver > doing this. In this case the SX150X is hot-plugged (on a slow > bus) so it needs to figure out all ranges at runtime anyway. Are you suggesting implementing a custom function for parsing "gpio-ranges" and calling it in pin_control_gpio_probe() instead of a generic helper? Or do you want to always map all the pin controller's pins to gpio pins as sx150x does? -Takahiro Akashi > Yours, > Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel