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

[Anup Patel]

> -----Original Message-----
> From: Simon Glass [mailto:sjg at chromium.org]
> Sent: Thursday, January 31, 2019 3:34 PM
> To: Alexander Graf <agraf at suse.de>
> Cc: 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>; 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>; Michal Simek
> <michal.simek at xilinx.com>
> Subject: Re: [PATCH 04/11] net: macb: Fix clk API usage for RISC-V systems
> 
> Hi,
> 
> On Fri, 18 Jan 2019 at 06:35, Alexander Graf <agraf at suse.de> wrote:
> >
> >
> >
> > 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 :).
> 
> Yes, although putting an arch-specific #ifdef in a driver is not correct, so I
> think the onus would be on Michal to fix that if it were a problem.

Okay, I will drop the #ifndef and if required Michal can fix it for
Zynq SoC.

Regards,
Anup