[U-Boot] [PATCH] serial: ns16550: Fix serial output on Tegra186
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Reported-by: Alexandre Courbot acourbot@nvidia.com Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/serial/ns16550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 765499dab646..9c36dbe2a566 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -408,7 +408,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev) err = clk_get_rate(&clk); if (!IS_ERR_VALUE(err)) plat->clock = err; - } else if (err != -ENODEV && err != -ENOSYS) { + } else if (err != -ENOENT && err != -ENOSYS) { debug("ns16550 failed to get clock\n"); return err; }
On 09/30/2016 05:46 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Tested-by: Alexandre Courbot acourbot@nvidia.com
I sent a similar patch ~10 minutes before this one, but Thierry's commit message is clearer than mine (and his handling of -ENODEV probably more correct as well), so let's go with this version!
On Friday, 30 September 2016 17:53:38 BST Alexandre Courbot wrote:
On 09/30/2016 05:46 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Tested-by: Alexandre Courbot acourbot@nvidia.com
I sent a similar patch ~10 minutes before this one, but Thierry's commit message is clearer than mine (and his handling of -ENODEV probably more correct as well), so let's go with this version!
Hi Thierry & Alexandre,
Apologies for the breakage!
If a DT contains a clock & the UART node references it by phandle then I believe clk_get_by_index() could return -ENODEV via uclass_get_device_by_of_offset & uclass_find_device_by_of_offset. So we probably need to handle both -ENOENT for the "no clocks property" case & - ENODEV for the "clocks property but no driver" case, as Alexandre's patch does?
Thanks, Paul
Hi Paul,
On 09/30/2016 06:47 PM, Paul Burton wrote:
- PGP Signed by an unknown key
On Friday, 30 September 2016 17:53:38 BST Alexandre Courbot wrote:
On 09/30/2016 05:46 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Tested-by: Alexandre Courbot acourbot@nvidia.com
I sent a similar patch ~10 minutes before this one, but Thierry's commit message is clearer than mine (and his handling of -ENODEV probably more correct as well), so let's go with this version!
Hi Thierry & Alexandre,
Apologies for the breakage!
If a DT contains a clock & the UART node references it by phandle then I believe clk_get_by_index() could return -ENODEV via uclass_get_device_by_of_offset & uclass_find_device_by_of_offset. So we probably need to handle both -ENOENT for the "no clocks property" case & - ENODEV for the "clocks property but no driver" case, as Alexandre's patch does?
But if a DT contains a clock, shouldn't it simply take precedence over the legacy method? (not very familiar with this code so my comment may not make sense).
IOW, do we have a case where clk_get_by_index() returns -ENODEV and fdtdec_get_int() returns a valid clock?
On Fri, Sep 30, 2016 at 10:47:38AM +0100, Paul Burton wrote:
On Friday, 30 September 2016 17:53:38 BST Alexandre Courbot wrote:
On 09/30/2016 05:46 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Tested-by: Alexandre Courbot acourbot@nvidia.com
I sent a similar patch ~10 minutes before this one, but Thierry's commit message is clearer than mine (and his handling of -ENODEV probably more correct as well), so let's go with this version!
Hi Thierry & Alexandre,
Apologies for the breakage!
If a DT contains a clock & the UART node references it by phandle then I believe clk_get_by_index() could return -ENODEV via uclass_get_device_by_of_offset & uclass_find_device_by_of_offset. So we probably need to handle both -ENOENT for the "no clocks property" case & - ENODEV for the "clocks property but no driver" case, as Alexandre's patch does?
Indeed. I thought I had checked all possible return values, but I've obviously been blind. Alex's patch looks more correct, though I'm beginning to wonder if there's a better way to achieve this than keep adding whatever error codes happen to be special cases.
How about we just leave out the else completely? At this point it seems like any failure to obtain a valid clock would best be dealt with by falling back to clock-frequency or the one specified in the config.
Are there even other errors besides -ENODEV, -ENOENT and -ENOSYS that we could get here?
Thierry
On 09/30/2016 02:46 AM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Tested-by: Stephen Warren swarren@nvidia.com
On 10/03/2016 09:51 AM, Stephen Warren wrote:
On 09/30/2016 02:46 AM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
For Tegra186 there are currently no UART clocks wired up in device tree. This exposes a regression introduced in commit 50fce1d5d874 ("serial: ns16550: Support clocks via phandle"), which causes the p2771-0000-500 board (and probably any Tegra186-based board as well) to fail to boot.
The reason is that if no clocks property exists, then clk_get_by_index() returns -ENOENT (via fdtdec_parse_phandle_with_args()) rather than -ENODEV as the above-mentioned commit expects.
Fix this by checking for the right error code.
Tested-by: Stephen Warren swarren@nvidia.com
Actually, I take that back. This works great for Tegra186 but breaks earlier Tegra SoCs.
participants (4)
-
Alexandre Courbot -
Paul Burton -
Stephen Warren -
Thierry Reding