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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html