From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757564AbcK3OKJ (ORCPT ); Wed, 30 Nov 2016 09:10:09 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:37929 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757185AbcK3OJ7 (ORCPT ); Wed, 30 Nov 2016 09:09:59 -0500 Date: Wed, 30 Nov 2016 15:09:52 +0100 From: Gary Bisson To: Fabio Estevam Cc: Linus Walleij , Shawn Guo , Peng Fan , "linux-gpio@vger.kernel.org" , linux-kernel , Pantelis Antoniou , Vladimir Zapolskiy Subject: Re: [RFC] pinctrl: imx: use radix trees for groups and functions Message-ID: <20161130140952.cjl3fxab2lejrh5m@t450s.lan> References: <20161124233824.17424-1-gary.bisson@boundarydevices.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Fabio, All, On Wed, Nov 30, 2016 at 11:19:53AM -0200, Fabio Estevam wrote: > Hi Gary, > > On Thu, Nov 24, 2016 at 9:38 PM, Gary Bisson > wrote: > > This change is inspired from the pinctrl-single architecture. > > > > The problem with current implementation is that it isn't possible > > to add/remove functions and/or groups dynamically. The radix tree > > offers an easy way to do so. The intent is to offer a follow-up > > patch later that will enable the use of pinctrl nodes in dt-overlays. > > Maybe you could re-post it including the follow-up patch that allows > the usage of pinctrl nodes in dt-overlays? > > This way we can have a better picture of the final result. Sure. But before doing so I'd like to share my findings after adding the of_notifier: 1- Needs to remove the const qualifier of info, otherwise each member is read-only and can't be updated. Hope everybody is ok with that, but I don't see any other option to be able to increase the number of functions/groups. 2- The way to get groups from a function (get_function_groups) makes it hard to modify the number of groups of a specific function. Indeed it would require re-alloc'ing the char**. Instead, it'd be best to make a function for each group like it is done in pinctrl-single. In this case, the function name would be the same as its single group name. Then it is easy to add functions. This approach looks good, IMO, when a device tree is using the "new iomux scheme" since there's no real function. For the "legacy" scheme it can be confusing since there are functions in the device tree (each iomux sub-node). Is the legacy iomux scheme here to stay or is there a plan to modify every device tree to comply to the new layout? The other solution I came up with was that each overlay must have its own function and cannot expand an existing function. This is an unnecessary limitation in my opinion but I wanted to discuss it here before submitting any of the two solutions. My testing was done adding a dt-overlay pinctrl node, currently I'm not sure how to cleanly remove a pinctrl function/group. It is not supported in any of the pinctrl for now, but maybe it'd be a good time to think of it. Let me know if you have any question. Regards, Gary