Re: [PATCH 32/45] usb: ohci-at91: Check result of clk_get()

[Sam Ravnborg]

On Tue, Mar 07, 2017 at 01:21:40PM -0800, Andrey Smirnov wrote:
> On Tue, Mar 7, 2017 at 8:35 AM, Sam Ravnborg  wrote:
> > On Mon, Mar 06, 2017 at 02:53:43PM -0800, Andrey Smirnov wrote:
> >> Signed-off-by: Andrey Smirnov 
> >> ---
> >>  drivers/usb/host/ohci-at91.c | 9 +
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> >> index 0f5c8f1..c70d898 100644
> >> --- a/drivers/usb/host/ohci-at91.c
> >> +++ b/drivers/usb/host/ohci-at91.c
> >> @@ -47,7 +47,16 @@ static int at91_ohci_probe(struct device_d *dev)
> >>   struct ohci_regs __iomem *regs = (struct ohci_regs __iomem 
> >> *)dev->resource[0].start;
> >>
> >>   iclk = clk_get(NULL, "ohci_clk");
> >> + if (IS_ERR(iclk)) {
> >> + dev_err(dev, "Failed to get 'iclk'\n");
> > iclk is the internal name. Woudl it make more sense to use "ohci_clk" here?
> >
> 
> I was looking at the corresponding kernel driver and that's where I
> got the name for the variable. If you think 'ohci_clk' is a better
> name I have no problem changing it.

It is not something I have strong opinions about.
But I later noticed you sued "interface clk" and "function clk".
So it would be good to be consistent.

Even if the kernel is not consistent we can do better in barebox.

Sam

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 32/45] usb: ohci-at91: Check result of clk_get()

[Andrey Smirnov]

On Tue, Mar 7, 2017 at 8:35 AM, Sam Ravnborg  wrote:
> On Mon, Mar 06, 2017 at 02:53:43PM -0800, Andrey Smirnov wrote:
>> Signed-off-by: Andrey Smirnov 
>> ---
>>  drivers/usb/host/ohci-at91.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
>> index 0f5c8f1..c70d898 100644
>> --- a/drivers/usb/host/ohci-at91.c
>> +++ b/drivers/usb/host/ohci-at91.c
>> @@ -47,7 +47,16 @@ static int at91_ohci_probe(struct device_d *dev)
>>   struct ohci_regs __iomem *regs = (struct ohci_regs __iomem 
>> *)dev->resource[0].start;
>>
>>   iclk = clk_get(NULL, "ohci_clk");
>> + if (IS_ERR(iclk)) {
>> + dev_err(dev, "Failed to get 'iclk'\n");
> iclk is the internal name. Woudl it make more sense to use "ohci_clk" here?
>

I was looking at the corresponding kernel driver and that's where I
got the name for the variable. If you think 'ohci_clk' is a better
name I have no problem changing it.

>> + return PTR_ERR(iclk);
>> + }
>> +
>>   fclk = clk_get(NULL, "uhpck");
>> + if (IS_ERR(fclk)) {
>> + dev_err(dev, "Failed to get 'fclk'\n");
> Likewise - uhpck?

Ditto here.

Thanks,
Andrey Smirnov

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 32/45] usb: ohci-at91: Check result of clk_get()

[Sam Ravnborg]

On Mon, Mar 06, 2017 at 02:53:43PM -0800, Andrey Smirnov wrote:
> Signed-off-by: Andrey Smirnov 
> ---
>  drivers/usb/host/ohci-at91.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 0f5c8f1..c70d898 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -47,7 +47,16 @@ static int at91_ohci_probe(struct device_d *dev)
>   struct ohci_regs __iomem *regs = (struct ohci_regs __iomem 
> *)dev->resource[0].start;
>  
>   iclk = clk_get(NULL, "ohci_clk");
> + if (IS_ERR(iclk)) {
> + dev_err(dev, "Failed to get 'iclk'\n");
iclk is the internal name. Woudl it make more sense to use "ohci_clk" here?

> + return PTR_ERR(iclk);
> + }
> +
>   fclk = clk_get(NULL, "uhpck");
> + if (IS_ERR(fclk)) {
> + dev_err(dev, "Failed to get 'fclk'\n");
Likewise - uhpck?


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 32/45] usb: ohci-at91: Check result of clk_get()

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov 
---
 drivers/usb/host/ohci-at91.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 0f5c8f1..c70d898 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -47,7 +47,16 @@ static int at91_ohci_probe(struct device_d *dev)
struct ohci_regs __iomem *regs = (struct ohci_regs __iomem 
*)dev->resource[0].start;
 
iclk = clk_get(NULL, "ohci_clk");
+   if (IS_ERR(iclk)) {
+   dev_err(dev, "Failed to get 'iclk'\n");
+   return PTR_ERR(iclk);
+   }
+
fclk = clk_get(NULL, "uhpck");
+   if (IS_ERR(fclk)) {
+   dev_err(dev, "Failed to get 'fclk'\n");
+   return PTR_ERR(fclk);
+   }
 
/*
 * Start the USB clocks.
-- 
2.9.3


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox