diff mbox series

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

Message ID 20190117103748.36613-5-anup.patel@wdc.com
State Superseded
Delegated to: Andes
Headers show
Series SiFive FU540 Support | expand

Commit Message

Anup Patel Jan. 17, 2019, 10:38 a.m. UTC 
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@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 drivers/net/macb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Alexander Graf Jan. 17, 2019, 6:07 p.m. UTC | #1 
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@wdc.com>
> Reviewed-by: Bin Meng <bmeng.cn@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?


Alex

> -	if (ret)
> +	if (ret && ret != -ENOSYS)
>   		return ret;
>   #endif
>
Anup Patel Jan. 18, 2019, 6:05 a.m. UTC | #2 
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Thursday, January 17, 2019 11:37 PM
> To: Anup Patel <Anup.Patel@wdc.com>; Rick Chen <rick@andestech.com>;
> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
> Masahiro Yamada <yamada.masahiro@socionext.com>; Simon Glass
> <sjg@chromium.org>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> boot@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@wdc.com>
> > Reviewed-by: Bin Meng <bmeng.cn@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.

In all other cases, we use generic <clk.h> which provides
clk instances using DT based clk drivers.

Regards,
Anup
Alexander Graf Jan. 18, 2019, 11:35 a.m. UTC | #3 
> Am 18.01.2019 um 07:05 schrieb Anup Patel <Anup.Patel@wdc.com>:
> 
> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Thursday, January 17, 2019 11:37 PM
>> To: Anup Patel <Anup.Patel@wdc.com>; Rick Chen <rick@andestech.com>;
>> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
>> <joe.hershberger@ni.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
>> Masahiro Yamada <yamada.masahiro@socionext.com>; Simon Glass
>> <sjg@chromium.org>
>> Cc: Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
>> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
>> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
>> boot@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@wdc.com>
>>> Reviewed-by: Bin Meng <bmeng.cn@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?


Alex

> 
> In all other cases, we use generic <clk.h> which provides
> clk instances using DT based clk drivers.
> 
> Regards,
> Anup
Anup Patel Jan. 18, 2019, 1:28 p.m. UTC | #4 
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, January 18, 2019 5:05 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Rick Chen <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>;
> Joe Hershberger <joe.hershberger@ni.com>; Lukas Auer
> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
> <yamada.masahiro@socionext.com>; Simon Glass <sjg@chromium.org>;
> Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> boot@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@wdc.com>:
> >
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Thursday, January 17, 2019 11:37 PM
> >> To: Anup Patel <Anup.Patel@wdc.com>; Rick Chen
> <rick@andestech.com>;
> >> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
> >> <joe.hershberger@ni.com>; Lukas Auer
> >> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
> >> <yamada.masahiro@socionext.com>; Simon Glass <sjg@chromium.org>
> >> Cc: Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> >> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> >> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> >> boot@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@wdc.com>
> >>> Reviewed-by: Bin Meng <bmeng.cn@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.

Regards,
Anup
Alexander Graf Jan. 18, 2019, 1:35 p.m. UTC | #5 
On 18.01.19 14:28, Anup Patel wrote:
> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Friday, January 18, 2019 5:05 PM
>> To: Anup Patel <Anup.Patel@wdc.com>
>> Cc: Rick Chen <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>;
>> Joe Hershberger <joe.hershberger@ni.com>; Lukas Auer
>> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
>> <yamada.masahiro@socionext.com>; Simon Glass <sjg@chromium.org>;
>> Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
>> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
>> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
>> boot@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@wdc.com>:
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Thursday, January 17, 2019 11:37 PM
>>>> To: Anup Patel <Anup.Patel@wdc.com>; Rick Chen
>> <rick@andestech.com>;
>>>> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
>>>> <joe.hershberger@ni.com>; Lukas Auer
>>>> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
>>>> <yamada.masahiro@socionext.com>; Simon Glass <sjg@chromium.org>
>>>> Cc: Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
>>>> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
>>>> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
>>>> boot@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@wdc.com>
>>>>> Reviewed-by: Bin Meng <bmeng.cn@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
Simon Glass Jan. 31, 2019, 10:04 a.m. UTC | #6 
Hi,

On Fri, 18 Jan 2019 at 06:35, Alexander Graf <agraf@suse.de> wrote:
>
>
>
> On 18.01.19 14:28, Anup Patel wrote:
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Friday, January 18, 2019 5:05 PM
> >> To: Anup Patel <Anup.Patel@wdc.com>
> >> Cc: Rick Chen <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>;
> >> Joe Hershberger <joe.hershberger@ni.com>; Lukas Auer
> >> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
> >> <yamada.masahiro@socionext.com>; Simon Glass <sjg@chromium.org>;
> >> Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> >> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> >> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> >> boot@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@wdc.com>:
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Alexander Graf [mailto:agraf@suse.de]
> >>>> Sent: Thursday, January 17, 2019 11:37 PM
> >>>> To: Anup Patel <Anup.Patel@wdc.com>; Rick Chen
> >> <rick@andestech.com>;
> >>>> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
> >>>> <joe.hershberger@ni.com>; Lukas Auer
> >>>> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
> >>>> <yamada.masahiro@socionext.com>; Simon Glass <sjg@chromium.org>
> >>>> Cc: Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> >>>> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> >>>> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> >>>> boot@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@wdc.com>
> >>>>> Reviewed-by: Bin Meng <bmeng.cn@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.

Regards,
Simon
Anup Patel Feb. 5, 2019, 11:17 a.m. UTC | #7 
> -----Original Message-----
> From: Simon Glass [mailto:sjg@chromium.org]
> Sent: Thursday, January 31, 2019 3:34 PM
> To: Alexander Graf <agraf@suse.de>
> Cc: Anup Patel <Anup.Patel@wdc.com>; Rick Chen <rick@andestech.com>;
> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
> Masahiro Yamada <yamada.masahiro@socionext.com>; Palmer Dabbelt
> <palmer@sifive.com>; Paul Walmsley <paul.walmsley@sifive.com>; Atish
> Patra <Atish.Patra@wdc.com>; Christoph Hellwig <hch@infradead.org>; U-
> Boot Mailing List <u-boot@lists.denx.de>; Michal Simek
> <michal.simek@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@suse.de> wrote:
> >
> >
> >
> > On 18.01.19 14:28, Anup Patel wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Alexander Graf [mailto:agraf@suse.de]
> > >> Sent: Friday, January 18, 2019 5:05 PM
> > >> To: Anup Patel <Anup.Patel@wdc.com>
> > >> Cc: Rick Chen <rick@andestech.com>; Bin Meng
> <bmeng.cn@gmail.com>;
> > >> Joe Hershberger <joe.hershberger@ni.com>; Lukas Auer
> > >> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
> > >> <yamada.masahiro@socionext.com>; Simon Glass
> <sjg@chromium.org>;
> > >> Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> > >> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> > >> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> > >> boot@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@wdc.com>:
> > >>>
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Alexander Graf [mailto:agraf@suse.de]
> > >>>> Sent: Thursday, January 17, 2019 11:37 PM
> > >>>> To: Anup Patel <Anup.Patel@wdc.com>; Rick Chen
> > >> <rick@andestech.com>;
> > >>>> Bin Meng <bmeng.cn@gmail.com>; Joe Hershberger
> > >>>> <joe.hershberger@ni.com>; Lukas Auer
> > >>>> <lukas.auer@aisec.fraunhofer.de>; Masahiro Yamada
> > >>>> <yamada.masahiro@socionext.com>; Simon Glass
> <sjg@chromium.org>
> > >>>> Cc: Palmer Dabbelt <palmer@sifive.com>; Paul Walmsley
> > >>>> <paul.walmsley@sifive.com>; Atish Patra <Atish.Patra@wdc.com>;
> > >>>> Christoph Hellwig <hch@infradead.org>; U-Boot Mailing List <u-
> > >>>> boot@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@wdc.com>
> > >>>>> Reviewed-by: Bin Meng <bmeng.cn@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
diff mbox series

Patch

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 (ret)
+	if (ret && ret != -ENOSYS)
 		return ret;
 #endif