On 25/02/2025 08:42, wangweidong.a@xxxxxxxxxx wrote: > Thank you very much for your review > > On Fri, Feb 23, 2025 at 12:46:14 +0100, krzk@xxxxxxxxxx wrote: >> On Fri, Feb 21, 2025 at 06:26:23PM +0800, wangweidong.a@xxxxxxxxxx wrote: >>> + >>> +static void aw88166_hw_reset(struct aw88166 *aw88166) >>> +{ >>> + if (aw88166->reset_gpio) { >>> + gpiod_set_value_cansleep(aw88166->reset_gpio, 1); >>> + usleep_range(AW88166_1000_US, AW88166_1000_US + 10); >>> + gpiod_set_value_cansleep(aw88166->reset_gpio, 0); >>> + usleep_range(AW88166_1000_US, AW88166_1000_US + 10); >>> + gpiod_set_value_cansleep(aw88166->reset_gpio, 1); > >> Why do you keep reset as active after reset? How is it suppose to work? > > The gpio port of the AW88166 is reset when it is low. The value here is not value on the pin, but logical value for Linux. 1 means make it active. > So it's working now, I will modify it as follows: Really? Your DTS example has ACTIVE_LOW, so with combination of above it cannot work. Unless this is wrong or your DTS is wrong or this is not reset but "enable" pin etc. Best regards, Krzysztof