Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753308AbbEKJg7 (ORCPT ); Mon, 11 May 2015 05:36:59 -0400 Received: from mail-yk0-f178.google.com ([209.85.160.178]:35106 "EHLO mail-yk0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752955AbbEKJg4 (ORCPT ); Mon, 11 May 2015 05:36:56 -0400 MIME-Version: 1.0 X-Originating-IP: [95.23.52.107] In-Reply-To: <20150428141738.16243.18377.stgit@localhost> References: <20150428141736.16243.57292.stgit@localhost> <20150428141738.16243.18377.stgit@localhost> Date: Mon, 11 May 2015 11:36:55 +0200 Message-ID: Subject: Re: [PATCH 1/2] Regulator: mc34708: Add DT binding documentation From: Javier Martinez Canillas To: Martin Fuzzey Cc: Mark Brown , Liam Girdwood , Rob Herring , "devicetree@vger.kernel.org" , Linux Kernel , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1081 Lines: 33 Hello Martin, I haven't looked at your regulator driver in detail so this is not a full review but I just noticed something so I wanted mention. > + > + > +Example: > +&i2c3 { > + pmic: mc34708@08 { > + compatible = "fsl,mc34708"; > + reg = <0x08>; > + regulators { > +#define PMIC_REGMODE_SW_PFM 1 > +#define PMIC_REGMODE_SW_AUTO 2 > +#define PMIC_REGMODE_SW_PWM 3 > +#define PMIC_REGMODE_LDO_LP 1 > +#define PMIC_REGMODE_LDO_NORMAL 2 > + I see that you are defining these constants in your driver too (albeit with slightly different names). These kind of constants that are shared between the driver implementation and the Device Tree source files should be added to include/dt-bindings/regulator/ and included in both the driver and the DTS. Best regards, Javier -- 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/