Thread (14 messages) 14 messages, 3 authors, 2014-10-21

Re: [PATCH V2 4/9] Documentation: DT: Add entries for bcm63xx UART

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2014-10-20 22:53:53
Also in: linux-devicetree, linux-serial

On 10/20/2014 02:25 PM, Arnd Bergmann wrote:
On Monday 20 October 2014 23:20:08 Arnd Bergmann wrote:
quoted
In this example, the clock output name of the clock provider is
the same as the clock input of the consumer, that is almost always
a bug and would not be a good example at all.
Ah, found the bug: the MIPS code is written to ignore the device
and just look up a global clock name:

struct clk *clk_get(struct device *dev, const char *id)
{
        if (!strcmp(id, "enet0"))
                return &clk_enet0;
        if (!strcmp(id, "enet1"))
                return &clk_enet1;
        if (!strcmp(id, "enetsw"))
                return &clk_enetsw;
        if (!strcmp(id, "ephy"))
                return &clk_ephy;
        if (!strcmp(id, "usbh"))
                return &clk_usbh;
        if (!strcmp(id, "usbd"))
                return &clk_usbd;
        if (!strcmp(id, "spi"))
                return &clk_spi;
        if (!strcmp(id, "hsspi"))
                return &clk_hsspi;
        if (!strcmp(id, "xtm"))
                return &clk_xtm;
        if (!strcmp(id, "periph"))
                return &clk_periph;
        if ((BCMCPU_IS_3368() || BCMCPU_IS_6358()) && !strcmp(id, "pcm"))
                return &clk_pcm;
        if ((BCMCPU_IS_6362() || BCMCPU_IS_6368()) && !strcmp(id, "ipsec"))
                return &clk_ipsec;
        if ((BCMCPU_IS_6328() || BCMCPU_IS_6362()) && !strcmp(id, "pcie"))
                return &clk_pcie;
        return ERR_PTR(-ENOENT);
}

This should be changed to use the drivers/clk/clkdev.c lookup code if
you want to share drivers between architectures.
For now, I suppose that s simple fix could be to use an anonymous clock
request when probed via DT. This code you quote dates from 2008 when
there was no clkdev in the kernel at all. So something like this would
probably do it for now:
diff --git a/drivers/tty/serial/bcm63xx_uart.c
b/drivers/tty/serial/bcm63xx_uart.c
index e0b87d507670..1b914b85dd31 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -819,7 +819,7 @@ static int bcm_uart_probe(struct platform_device *pdev)
        if (!res_irq)
                return -ENODEV;

-       clk = clk_get(&pdev->dev, "periph");
+       clk = clk_get(&pdev->dev, pdev->dev.of_node ? NULL : "periph");
        if (IS_ERR(clk))
                return -ENODEV;


In particular, the "enet0"/"enet1" clock name makes no sense -- the
clock input name should be independent of the instance, aside from
the question of which output of the provider it is wired up to.

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help