Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754272AbdCMQlk (ORCPT ); Mon, 13 Mar 2017 12:41:40 -0400 Received: from vps0.lunn.ch ([178.209.37.122]:46213 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754161AbdCMQl3 (ORCPT ); Mon, 13 Mar 2017 12:41:29 -0400 Date: Mon, 13 Mar 2017 17:41:18 +0100 From: Andrew Lunn To: sean.wang@mediatek.com Cc: f.fainelli@gmail.com, vivien.didelot@savoirfairelinux.com, matthias.bgg@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, davem@davemloft.net, Landen.Chao@mediatek.com, keyhaede@gmail.com, objelf@gmail.com Subject: Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch Message-ID: <20170313164118.GE3953@lunn.ch> References: <1489421488-300-1-git-send-email-sean.wang@mediatek.com> <1489421488-300-5-git-send-email-sean.wang@mediatek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1489421488-300-5-git-send-email-sean.wang@mediatek.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1381 Lines: 47 Hi Sean Just looking at the GPIO handling at the moment. > + /* Reset whole chip through gpio pin or > + * memory-mapped registers for different > + * type of hardware > + */ > + if (priv->mcm) { > + regmap_update_bits(priv->ethsys, SYSC_REG_RSTCTRL, > + RESET_MCM, RESET_MCM); > + usleep_range(1000, 1100); > + regmap_update_bits(priv->ethsys, SYSC_REG_RSTCTRL, > + RESET_MCM, ~RESET_MCM); > + } else { > + gpio_direction_output(priv->reset, 0); > + usleep_range(1000, 1100); > + gpio_set_value(priv->reset, 1); > + } .... > + /* Not MCM that indicates switch works as the remote standalone > + * integrated circuit so the GPIO pin would be used to complete > + * the reset, otherwise memory-mapped register accessing used > + * through syscon provides in the case of MCM. > + */ > + if (!priv->mcm) { > + priv->reset = of_get_named_gpio(dn, "mediatek,reset-pin", 0); > + if (!gpio_is_valid(priv->reset)) > + return priv->reset; > + > + ret = devm_gpio_request_one(&mdiodev->dev, > + priv->reset, GPIOF_OUT_INIT_LOW, > + "mediatek,reset-pin"); > + if (ret < 0) { > + dev_err(&mdiodev->dev, > + "fail to devm_gpio_request reset\n"); > + return ret; > + } > + } You are not handling the flags part of the GPIO binding. It is better to use devm_gpiod_ API calls, which will handle the active low flags for you. Andrew