From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756234AbcKKIgb (ORCPT ); Fri, 11 Nov 2016 03:36:31 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:47771 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756213AbcKKIg3 (ORCPT ); Fri, 11 Nov 2016 03:36:29 -0500 Date: Fri, 11 Nov 2016 09:36:27 +0100 From: Maxime Ripard To: Chen-Yu Tsai Cc: Linus Walleij , linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com Subject: Re: [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware Message-ID: <20161111083627.yooctcxqs4ow7sn3@lukather> References: <20161111024455.16883-1-wens@csie.org> <20161111024455.16883-3-wens@csie.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6zko3kx573tyunym" Content-Disposition: inline In-Reply-To: <20161111024455.16883-3-wens@csie.org> User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --6zko3kx573tyunym Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 11, 2016 at 10:44:54AM +0800, Chen-Yu Tsai wrote: > The sunxi pinctrl driver only caches whatever pinconf setting was last > set on a given pingroup. This is not particularly helpful, nor is it > correct. >=20 > Fix this by actually reading the hardware registers and returning > the correct results or error codes. Also filter out unsupported > pinconf settings. Since this driver has a peculiar setup of 1 pin > per group, we can support both pin and pingroup pinconf setting > read back with the same code. The sunxi_pconf_reg helper and code > structure is inspired by pinctrl-msm. >=20 > With this done we can also claim to support generic pinconf, by > setting .is_generic =3D true in pinconf_ops. >=20 > Signed-off-by: Chen-Yu Tsai > --- > drivers/pinctrl/sunxi/pinctrl-sunxi.c | 84 +++++++++++++++++++++++++++++= ++++-- > 1 file changed, 81 insertions(+), 3 deletions(-) >=20 > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunx= i/pinctrl-sunxi.c > index e04edda8629d..3e9f7c675d36 100644 > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > @@ -438,15 +438,91 @@ static const struct pinctrl_ops sunxi_pctrl_ops =3D= { > .get_group_pins =3D sunxi_pctrl_get_group_pins, > }; > =20 > +static int sunxi_pconf_reg(unsigned pin, enum pin_config_param param, > + u32 *offset, u32 *shift, u32 *mask) > +{ > + switch (param) { > + case PIN_CONFIG_DRIVE_STRENGTH: > + *offset =3D sunxi_dlevel_reg(pin); > + *shift =3D sunxi_dlevel_offset(pin); > + *mask =3D DLEVEL_PINS_MASK; > + break; > + > + case PIN_CONFIG_BIAS_PULL_UP: > + case PIN_CONFIG_BIAS_PULL_DOWN: > + case PIN_CONFIG_BIAS_DISABLE: > + *offset =3D sunxi_pull_reg(pin); > + *shift =3D sunxi_pull_offset(pin); > + *mask =3D PULL_PINS_MASK; > + break; > + > + default: > + return -ENOTSUPP; > + } > + > + return 0; > +} > + > +static int sunxi_pconf_get(struct pinctrl_dev *pctldev, unsigned pin, > + unsigned long *config) > +{ > + struct sunxi_pinctrl *pctl =3D pinctrl_dev_get_drvdata(pctldev); > + enum pin_config_param param =3D pinconf_to_config_param(*config); > + u32 offset, shift, mask, val; > + u16 arg; > + int ret; > + > + pin -=3D pctl->desc->pin_base; > + > + ret =3D sunxi_pconf_reg(pin, param, &offset, &shift, &mask); > + if (ret < 0) > + return ret; > + > + val =3D (readl(pctl->membase + offset) >> shift) & mask; > + > + switch (pinconf_to_config_param(*config)) { > + case PIN_CONFIG_DRIVE_STRENGTH: > + arg =3D (val + 1) * 10; > + break; > + > + case PIN_CONFIG_BIAS_PULL_UP: > + if (val !=3D SUN4I_PINCTRL_PULL_UP) > + return -EINVAL; > + arg =3D 1; /* hardware is weak pull-up */ > + break; > + > + case PIN_CONFIG_BIAS_PULL_DOWN: > + if (val !=3D SUN4I_PINCTRL_PULL_DOWN) > + return -EINVAL; > + arg =3D 1; /* hardware is weak pull-down */ > + break; > + > + case PIN_CONFIG_BIAS_DISABLE: > + if (val !=3D SUN4I_PINCTRL_NO_PULL) > + return -EINVAL; > + arg =3D 0; > + break; > + > + default: > + /* sunxi_pconf_reg should catch anything unsupported */ > + WARN_ON(1); > + return -ENOTSUPP; > + } > + > + *config =3D pinconf_to_config_packed(param, arg); > + > + return 0; > +} > + > static int sunxi_pconf_group_get(struct pinctrl_dev *pctldev, > unsigned group, > unsigned long *config) > { > struct sunxi_pinctrl *pctl =3D pinctrl_dev_get_drvdata(pctldev); > + struct sunxi_pinctrl_group *g =3D &pctl->groups[group]; > =20 > - *config =3D pctl->groups[group].config; Do we still need this variable? Looking at the code, it doesn't look that way, and we can remove the caching in the _group_set function and the variable itslef in the sunxi_pincttrl_group structure. Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --6zko3kx573tyunym Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYJYMLAAoJEBx+YmzsjxAg2z4P/A6w2rfX2X6xauL2tQgFddv3 Oma9HcxkXatKdJizdRB49jJzX9SbWYpnMqmIRLqOe3eS6IknAYn0Nwv4zPBeLZkM H+XUlfgRCZYxbvM6jDPtPJmhJyvFfvKzECgyHcSKRHM4JzsMlZaZsWhqko3v+qFM y2RsvDftyURb81g2pDySEpXXSml/v+qeksZRkNdt0VntRVavtNZpQ6+sz9eVVrfK cbMX31HOC9+OlP7tPNG5l+E+PcDLJlFQj/rSawCWuHCfXsteTmEvgAiReOIkYl/t Oj8Yjo2dqn/qRF1vSWBySmHpM4zyP85IwG7D3p54g8NSDgl7OCRGLp+bPdTiCH7l ct/3du/QSIYaoLgjX8moDxH9sfpg7GCyMeHO23teOW+C6pOL7yroSqyJDH6NcXbc NQWuqz1aqdc3yFsrVBxICw8Eyq3oD1hxFK1ReQE7wNOAcxZlqRTDGqReAsZ8FMka WEKq9xrdgPgBwz5EaU1BUtf+6E7A5Hmz5Iik0w6od+3drVuhKcrt7yUDF7E1AscS JiROH7mVGaZnsQoSbACtneCKCcjhfcIjGD3QQ4+aowf3sVhm1Lga6tewu2HW2cSl ZDqx8x3nqwLuyvU34AcbenGZTDy9X3lraEhSjUSIQZbk2YB/gT8SwPpmkR+oOQHj lYli5hYQo20O5F3t14HS =C+4W -----END PGP SIGNATURE----- --6zko3kx573tyunym--