From patchwork Thu Oct 9 15:16:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 398072 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9F2461400AA for ; Fri, 10 Oct 2014 02:16:16 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757408AbaJIPQP (ORCPT ); Thu, 9 Oct 2014 11:16:15 -0400 Received: from mail-ig0-f180.google.com ([209.85.213.180]:57763 "EHLO mail-ig0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755824AbaJIPQP (ORCPT ); Thu, 9 Oct 2014 11:16:15 -0400 Received: by mail-ig0-f180.google.com with SMTP id uq10so4521144igb.7 for ; Thu, 09 Oct 2014 08:16:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=CgWlor0khfoUMd+wC6okBtTqKEMqeO/fNims8cN+DS4=; b=cQNRYNrPp21PRJXQNWCccxv1h0GSZ4X48bHp9MbpxNLTDgZRCmq7unZuaM/YizO3UY znspzs/EpXx3JCMFtNpFuliouTJ+IxRF7DFXDvOnB+ghowSU5SJqGGr2nFEmI5QHxfl4 rC6x1lhvvTeexBX29yx0i5N+eqb98uYA6jWmcR3ofrpDZYH+ClIPjAzOk0sB9XukeR1f 5rUZNO0lwIifFrmHyJ2+ULfeVJ3EGOcatb6djRkP3AU+dVYXj0dw/2lKc7wfSXJPQaqY rVugmTqOjjFJgZAAiaxNoqUV8fybi5H1e0tWwwdEqBDtcJ7Jz6rP9pzSVviqKxp2H7YF syUg== X-Received: by 10.42.94.12 with SMTP id z12mr9196234icm.46.1412867774390; Thu, 09 Oct 2014 08:16:14 -0700 (PDT) Received: from localhost ([216.228.120.20]) by mx.google.com with ESMTPSA id au4sm22963613igc.3.2014.10.09.08.16.12 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Oct 2014 08:16:13 -0700 (PDT) Date: Thu, 9 Oct 2014 17:16:08 +0200 From: Thierry Reding To: Lothar =?utf-8?Q?Wa=C3=9Fmann?= Cc: linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Shawn Guo , Sascha Hauer , Tony Prisk Subject: Re: [PATCHv5 2/3] pwm: make the PWM_POLARITY flag in DTB optional Message-ID: <20141009151605.GA8818@ulmo.nvidia.com> References: <1412690134-13712-1-git-send-email-LW@KARO-electronics.de> <1412690134-13712-3-git-send-email-LW@KARO-electronics.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1412690134-13712-3-git-send-email-LW@KARO-electronics.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-pwm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org On Tue, Oct 07, 2014 at 03:55:33PM +0200, Lothar Waßmann wrote: > Change the pwm chip driver registration, so that a chip driver that > supports polarity inversion can still be used with DTBs that don't > provide the 'PWM_POLARITY' flag. > > This is done to provide polarity inversion support for the pwm-imx > driver without having to modify all existing DTS files. I don't like how this throws out the window the only sanity checking we have in place for the #pwm-cells property. As I understand it, the problem that you're trying to solve is one of backwards-compatibility where existing device trees have #pwm-cells = <2>, but the driver is extended to support flags as well. In that case, can we not simply make of_pwm_xlate_with_flags() support that case transparently? That is, if the driver sets .of_pwm_n_cells to 3, we can still support #pwm-cells = <2> and use the default (no) flags instead. Something like the below should do that (compile-tested only). Thierry diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 966497d10c6e..89a5e309b0a3 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -136,9 +136,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; + /* check that the driver supports a third cell for flags */ if (pc->of_pwm_n_cells < 3) return ERR_PTR(-EINVAL); + /* flags in the third cell are optional */ + if (args->args_count < 2) + return ERR_PTR(-EINVAL); + if (args->args[0] >= pc->npwm) return ERR_PTR(-EINVAL); @@ -148,10 +153,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) pwm_set_period(pwm, args->args[1]); - if (args->args[2] & PWM_POLARITY_INVERTED) - pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); - else - pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); + if (args->args_count > 2) { + if (args->args[2] & PWM_POLARITY_INVERTED) + pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); + else + pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); + } return pwm; } @@ -162,9 +169,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; + /* sanity check driver support */ if (pc->of_pwm_n_cells < 2) return ERR_PTR(-EINVAL); + /* all cells are required */ + if (args->args_count != pc->of_pwm_n_cells) + return ERR_PTR(-EINVAL); + if (args->args[0] >= pc->npwm) return ERR_PTR(-EINVAL); @@ -536,13 +548,6 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) goto put; } - if (args.args_count != pc->of_pwm_n_cells) { - pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name, - args.np->full_name); - pwm = ERR_PTR(-EINVAL); - goto put; - } - pwm = pc->of_xlate(pc, &args); if (IS_ERR(pwm)) goto put;