[U-Boot] [PATCH 04/11] net: macb: Fix clk API usage for RISC-V systems

[Alexander Graf]

On 18.01.19 14:28, Anup Patel wrote:
> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf at suse.de]
>> Sent: Friday, January 18, 2019 5:05 PM
>> To: Anup Patel <Anup.Patel at wdc.com>
>> Cc: Rick Chen <rick at andestech.com>; Bin Meng <bmeng.cn at gmail.com>;
>> Joe Hershberger <joe.hershberger at ni.com>; Lukas Auer
>> <lukas.auer at aisec.fraunhofer.de>; Masahiro Yamada
>> <yamada.masahiro at socionext.com>; Simon Glass <sjg at chromium.org>;
>> Palmer Dabbelt <palmer at sifive.com>; Paul Walmsley
>> <paul.walmsley at sifive.com>; Atish Patra <Atish.Patra at wdc.com>;
>> Christoph Hellwig <hch at infradead.org>; U-Boot Mailing List <u-
>> boot at lists.denx.de>
>> Subject: Re: [PATCH 04/11] net: macb: Fix clk API usage for RISC-V systems
>>
>>
>>
>>> Am 18.01.2019 um 07:05 schrieb Anup Patel <Anup.Patel at wdc.com>:
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf at suse.de]
>>>> Sent: Thursday, January 17, 2019 11:37 PM
>>>> To: Anup Patel <Anup.Patel at wdc.com>; Rick Chen
>> <rick at andestech.com>;
>>>> Bin Meng <bmeng.cn at gmail.com>; Joe Hershberger
>>>> <joe.hershberger at ni.com>; Lukas Auer
>>>> <lukas.auer at aisec.fraunhofer.de>; Masahiro Yamada
>>>> <yamada.masahiro at socionext.com>; Simon Glass <sjg at chromium.org>
>>>> Cc: Palmer Dabbelt <palmer at sifive.com>; Paul Walmsley
>>>> <paul.walmsley at sifive.com>; Atish Patra <Atish.Patra at wdc.com>;
>>>> Christoph Hellwig <hch at infradead.org>; U-Boot Mailing List <u-
>>>> boot at lists.denx.de>
>>>> Subject: Re: [PATCH 04/11] net: macb: Fix clk API usage for RISC-V
>>>> systems
>>>>
>>>>> On 01/17/2019 11:38 AM, Anup Patel wrote:
>>>>> This patch does following fixes in MACB ethernet driver for using it
>>>>> on RISC-V systems (particularly QEMU sifive_u
>>>>> machine):
>>>>> 1. asm/arch/clk.h is not available on RISC-V port so include
>>>>>    it only for non-RISC-V systems.
>>>>> 2. Don't fail in macb_enable_clk() if clk_enable() returns
>>>>>    -ENOSYS because we get -ENOSYS for fixed-rate clocks.
>>>>>
>>>>> Signed-off-by: Anup Patel <anup.patel at wdc.com>
>>>>> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
>>>>> ---
>>>>>  drivers/net/macb.c | 4 +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c index
>>>>> 94c89c762b..9a06b523cc 100644
>>>>> --- a/drivers/net/macb.c
>>>>> +++ b/drivers/net/macb.c
>>>>> @@ -38,7 +38,9 @@
>>>>>  #include <linux/mii.h>
>>>>>  #include <asm/io.h>
>>>>>  #include <asm/dma-mapping.h>
>>>>> +#ifndef CONFIG_RISCV
>>>>>  #include <asm/arch/clk.h>
>>>>> +#endif
>>>>>  #include <linux/errno.h>
>>>>>
>>>>>  #include "macb.h"
>>>>> @@ -1066,7 +1068,7 @@ static int macb_enable_clk(struct udevice
>> *dev)
>>>>>       */
>>>>>  #ifndef CONFIG_MACB_ZYNQ
>>>>>      ret = clk_enable(&clk);
>>>>
>>>> If clk.h is not available, who exports clk_enable() then; and why is
>>>> the included needed in the first place?
>>>
>>> For some of the ARM boards, clk instances are provided directly by
>>> arch/arm/mach-xyz sources. For such boards, asm/arch/clk.h is
>>> required. I think these boards should move to DT based clk drivers.
>>
>> Can you at least make this a positive #ifdef then rather than ifndef? We want
>> to isolate the odd case, not the normal one.
>>
>> Maybe you can even find a config option that isolates it further?
> 
> With the additional check "ret != -ENOSYS", we don't require the #ifndef but
> someone with Zynq SOC board need to confirm that removing #ifndef won't
> hurt.

In that case let's CC Michal :).


Alex