On 6/18/24 00:03, Hugo Villeneuve wrote:
On Fri, 14 Jun 2024 18:29:52 +0800
Hui Wang <hui.wang@xxxxxxxxxxxxx> wrote:
Hi Hui,
Some boards connect a GPIO to the reset pin, and the reset pin needs
...
+ struct gpio_desc *reset_gpio;
+
+ /*
+ * The reset input is active low, and flag GPIOD_OUT_HIGH ensures the
+ * GPIO is low once devm_gpiod_get_optional returns a valid gpio_desc.
+ */
I would replace all the above comments with:
/* Assert reset GPIO if defined and valid. */
The correct polarity is already defined by the device
tree reset-gpios entry, and can be high or low depending on the design
(ex: there can be an inverter between the CPU and the chip reset input,
etc).
Agree with that, I will change it in the v6.
+ reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(reset_gpio), "Failed to get reset GPIO\n");
+
...
+out_kthread:
kthread_stop(s->kworker_task);
out_clk:
--
2.34.1
I could not test the validity of the 3us delay since I do not have an
oscilloscope, but testing with a 10s delay instead and a
multimeter showed that it works ok. You can add my Tested-by tag:
Tested-by: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
And if you modify the comment as I suggested above, then you can add my
R-b tag:
Reviewed-by: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
OK. thanks.
