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

[Florian Fainelli <f.fainelli@gmail.com>]

On 10/20/2014 02:25 PM, Arnd Bergmann wrote:
> On Monday 20 October 2014 23:20:08 Arnd Bergmann wrote:
>>
>> 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
>