Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753392Ab3H0D5j (ORCPT ); Mon, 26 Aug 2013 23:57:39 -0400 Received: from avon.wwwdotorg.org ([70.85.31.133]:38703 "EHLO avon.wwwdotorg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752798Ab3H0D5h (ORCPT ); Mon, 26 Aug 2013 23:57:37 -0400 Message-ID: <521C23AF.2060309@wwwdotorg.org> Date: Mon, 26 Aug 2013 21:57:35 -0600 From: Stephen Warren User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: boris brezillon CC: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Rob Landley , Russell King , Linus Walleij , Jean-Christophe Plagniol-Villard , Jiri Kosina , Masanari Iida , Nicolas Ferre , Richard Genoud , Heiko Stuebner , James Hogan , devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf References: <1377379926-11163-1-git-send-email-b.brezillon@overkiz.com> <1377380259-11290-1-git-send-email-b.brezillon@overkiz.com> <521B8822.50304@wwwdotorg.org> <521B8D9B.5020809@overkiz.com> In-Reply-To: <521B8D9B.5020809@overkiz.com> X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2653 Lines: 71 On 08/26/2013 11:17 AM, boris brezillon wrote: > On 26/08/2013 18:53, Stephen Warren wrote: >> On 08/24/2013 03:37 PM, Boris BREZILLON wrote: >>> Add support for generic pin configuration to pinctrl-at91 driver. >>> diff --git >>> a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt >>> b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt >>> Required properties for iomux controller: >>> -- compatible: "atmel,at91rm9200-pinctrl" >>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl". >> You seem to also be adding a second chip name to the list here, which is >> more than the patch subject/description imply you're doing... > > This is an update of the documentation: > "atmel,at91sam9x5-pinctrl" compatible is already used in the pinctrl > driver but the documention > was not updated. > > But I agree, this should not be part of this series. > >>> + Add "generic-pinconf" to the compatible string list to use the >>> generic pin >>> + configuration syntax. >> "generic-pinconf" is too generic of a compatible value for this binding >> to define. >> >> Instead, I think you want to either: >> >> a) >> >> Use compatible="atmel,at91rm9200-pinctrl" for the old binding, >> use compatible="atmel,at91rm9200-pinctrl-generic" for the new binding >> >> or: >> >> b) >> >> Define Boolean property atmel,generic-pinconf (perhaps a better name >> could be chosen?). If it's not present, parse the node assuming the old >> binding. If it is present, parse the node assuming the new binding. >> > Okay. > > I thought this property string could be generic as it may concern other > drivers too > (in order to keep compatibility with old dt ABI and add support the > generic pinconf binding). > > Anyway, I prefer the first proposition. > > pinctrl single driver is already using these names: > > |compatible = "pinctrl-single" for non generic pinconf binding > ||compatible = "pinconf-single" ||for generic pinconf binding| > > So I think we should use something similar: > > |compatible = "atmel,at91xx-pinctrl" for non generic pinconf binding > ||compatible = "|||atmel,at91xx-|pinconf" ||for generic pinconf binding| > > What do you think ? Hmmm. It is a little odd to switch out the compatible value and invent a new binding for the same HW. Isn't it possible to define both sets of properties in the binding, and have drivers look for either? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/