在 2024/8/8 18:36, Krzysztof Kozlowski 写道:
On 08/08/2024 05:52, Shawn Lin wrote:
RK3576 contains a UFS controller, add init support fot it.
Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
...
+ err = clk_prepare_enable(host->ref_out_clk);
+ if (err)
+ return dev_err_probe(dev, err, "failed to enable ref out clock\n");
+
+ host->rst_gpio = devm_gpiod_get(&pdev->dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(host->rst_gpio)) {
+ dev_err_probe(&pdev->dev, PTR_ERR(host->rst_gpio),
+ "invalid reset-gpios property in node\n");
+ err = PTR_ERR(host->rst_gpio);
No. Look at your code above - you have return dev_err_probe, so logical
is that the syntax is err = dev_err_probe. Don't over-complicate the code.
Anyway, this is suspicious. You already have resets with four resets
(!!!) and you claim you have fifth reset - GPIO? This looks just wrong,
like you represent some properties which do not belong here.
Thanks for the feadback.
Yes, we have 4 resets for controller, and one gpio to reset the
device. It happened to be called reset-gpios in DT but can be
any name if you like it to be. I added reset-gpios as a required one
listed in dt-bindings file, in patch 2.
> Also, why do you leave the device in the reset state? Logical one
means
> - reset is asserted. This applies to ufs_rockchip_device_reset() as >
well
> - that's just wrong code.
No, I tested it in linux-net and the output is HIGH, and leave the
device in active state.
In dts, we add:
reset-gpios = <&gpio4 RK_PD0 GPIO_ACTIVE_HIGH>;
so gpiod_set_value_cansleep(host->rst_gpio, 1) -> output HIGH for
gpio4_d0. Based on your comment, we should change the dts to use
GPIO_ACTIVE_LOW and then gpiod_set_value_cansleep（host->rst_gpio, 0）
Both can work, however IMO, isn't logical one means HIGH level in line
more human readable?
Where is your DTS so we can validate it?
Will add it in the next version, as now the rk3576.dtsi is under
reviewed and UFS node was not added. So I was afraid to interfere
with that patch and just wanted to add incremental patch once
rk3576.dtsi got merged.
Best regards,
Krzysztof
