Hi Neil,

On Mon, Mar 4, 2019 at 11:40 AM Neil Armstrong [off-list ref] wrote:
[...]

+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>

there's a "regmap" include right above. this driver doesn't use syscon
so this include can be dropped

[...]

+static int phy_meson_g12a_usb2_exit(struct phy *phy)
+{
+       struct phy_meson_g12a_usb2_priv *priv = phy_get_drvdata(phy);
+
+       return reset_control_reset(priv->reset);

do you know whether we should reset_control_assert here instead of
reset_control_reset?
the probe function below already uses reset_control_deassert, so the
current implementation is inconsistent. in v1 you replied with "Maybe
it would be better, indeed." - if there's a reason why
reset_control_assert doesn't work here then I would like to have a
comment stating why

Apart from these two this is looking good!
Human readable BIT/GENMASK #defines for the register bits would be
nice, but I'm not sure if you have the details to add these.


Regards
Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel