Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh

2018-04-05 Thread Masahiro Yamada
2018-04-06 3:59 GMT+09:00 Jiri Olsa <jo...@redhat.com>:
> On Fri, Apr 06, 2018 at 12:50:00AM +0900, Masahiro Yamada wrote:
>> 2018-04-06 0:16 GMT+09:00 Jiri Olsa <jo...@kernel.org>:
>> > There's no need to pass LD* arguments to link-vmlinux.sh,
>> > because they are passed as variables. The only argument
>> > the link-vmlinux.sh supports is the 'clean' argument.
>> >
>> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
>> > ---
>>
>> Wrong.
>>
>> $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
>> exist here so that any change in them
>> invokes scripts/linkk-vmlinux.sh
>
> sry, I can't see that.. but it's just a side fix,
> which is actually not needed for the rest
>
> I'll check on more and address this separately


The link command is recorded in .vmlinux.cmd
then, it is checked by if_changed in the next rebuild
because link-vmlinux is invoked by $(call if_changed,...)

 +$(call if_changed,link-vmlinux)



> thanks,
> jirka
>
>>
>>
>>
>>
>> >  Makefile | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index d3300e46f925..a65a3919c6ad 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard 
>> > $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>> >
>> >  # Final link of vmlinux with optional arch pass after final link
>> >  cmd_link-vmlinux = \
>> > -   $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;    \
>> > +   $(CONFIG_SHELL) $< ;   \
>> > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>> >
>> >  vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
>>
>>
>>
>>
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada


Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh

2018-04-05 Thread Masahiro Yamada
2018-04-06 0:16 GMT+09:00 Jiri Olsa <jo...@kernel.org>:
> There's no need to pass LD* arguments to link-vmlinux.sh,
> because they are passed as variables. The only argument
> the link-vmlinux.sh supports is the 'clean' argument.
>
> Signed-off-by: Jiri Olsa <jo...@kernel.org>
> ---

Wrong.

$(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
exist here so that any change in them
invokes scripts/linkk-vmlinux.sh




>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index d3300e46f925..a65a3919c6ad 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard 
> $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>
>  # Final link of vmlinux with optional arch pass after final link
>  cmd_link-vmlinux = \
> -   $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
> +   $(CONFIG_SHELL) $< ;   \
> $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>
>  vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE






-- 
Best Regards
Masahiro Yamada


Re: [PATCH 01/10] net/sched: kconfig: Remove empty help texts

2018-01-31 Thread Masahiro Yamada
2018-02-01 0:32 GMT+09:00 David Miller <da...@davemloft.net>:
> From: Ulf Magnusson <ulfali...@gmail.com>
> Date: Tue, 30 Jan 2018 20:05:23 +0100
>
>> In preparation for adding a warning ("kconfig: Warn if help text is
>> blank"): https://lkml.org/lkml/2018/1/30/516
>>
>> Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
>
> Applied.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


I had applied the whole series into linux-kbuild.


Do you want to apply it to your tree?


-- 
Best Regards
Masahiro Yamada


Re: [PATCH] kbuild: make Makefile|Kbuild in each directory optional

2018-01-25 Thread Masahiro Yamada
2018-01-25 10:41 GMT+09:00 Jakub Kicinski <jakub.kicin...@netronome.com>:
> It is useful to be able to build single object files, e.g.:
> $ make net/sched/cls_flower.o W=1 C=2
>
> Currently kbuild does a hard include of a Kbuild or Makefile
> for directory where that object would reside.  Kbuild doesn't
> cater too well to multi-directory drivers, meaning such drivers
> will usually only use a single central Makefile.  This in turn
> means it will be impossible to build most of object files
> individually for such drivers.
>
> Make the include of $dir/{Makefile,Kbuild} optional.
>
> Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vanderme...@netronome.com>
> ---
> I must admit I have no idea whose tree I should send this to :(
> Could it go via net-next if no one on linux-kbuild objects?

This should be taken care of by linux-kbuild (and me).

For your specific problem, please fix netronome/nfp/Makefile


I'd like to take my time to think about this patch.

The single target is a bit compromised implementation
(it cannot handle subdir-ccflags-y correctly, for example)
I wonder if we should do this just for single target...





>  scripts/Makefile.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 47cddf32aeba..178864f877d5 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -42,7 +42,7 @@ save-cflags := $(CFLAGS)
>  # The filename Kbuild has precedence over Makefile
>  kbuild-dir := $(if $(filter /%,$(src)),$(src),$(srctree)/$(src))
>  kbuild-file := $(if $(wildcard 
> $(kbuild-dir)/Kbuild),$(kbuild-dir)/Kbuild,$(kbuild-dir)/Makefile)
> -include $(kbuild-file)
> +-include $(kbuild-file)
>
>  # If the save-* variables changed error out
>  ifeq ($(KBUILD_NOPEDANTIC),)
> --
> 2.15.1
>



-- 
Best Regards
Masahiro Yamada


[PATCH 0/2] Consolidate BUILD_BUG macros

2018-01-04 Thread Masahiro Yamada
Masahiro Yamada (2):
  genl_magic: remove own BUILD_BUG_ON*() defines
  build_bug.h: remove BUILD_BUG_ON_NULL()

 include/linux/build_bug.h   |  2 --
 include/linux/genl_magic_func.h | 12 +---
 net/ipv6/mcast.c|  8 
 3 files changed, 5 insertions(+), 17 deletions(-)

-- 
2.7.4



[PATCH 2/2] build_bug.h: remove BUILD_BUG_ON_NULL()

2018-01-04 Thread Masahiro Yamada
This macro is only used by net/ipv6/mcast.c, but there is no reason
why it must be BUILD_BUG_ON_NULL().

Replace it with BUILD_BUG_ON_ZERO(), and remove BUILD_BUG_ON_NULL()
definition from .

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 include/linux/build_bug.h | 2 --
 net/ipv6/mcast.c  | 8 
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index 3efed0d..43d1fd5 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -8,7 +8,6 @@
 #define __BUILD_BUG_ON_NOT_POWER_OF_2(n) (0)
 #define BUILD_BUG_ON_NOT_POWER_OF_2(n) (0)
 #define BUILD_BUG_ON_ZERO(e) (0)
-#define BUILD_BUG_ON_NULL(e) ((void *)0)
 #define BUILD_BUG_ON_INVALID(e) (0)
 #define BUILD_BUG_ON_MSG(cond, msg) (0)
 #define BUILD_BUG_ON(condition) (0)
@@ -28,7 +27,6 @@
  * aren't permitted).
  */
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
-#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:(-!!(e)); }))
 
 /*
  * BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 8446426..b28d4aa 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -65,10 +65,10 @@
 #include 
 
 /* Ensure that we have struct in6_addr aligned on 32bit word. */
-static void *__mld2_query_bugs[] __attribute__((__unused__)) = {
-   BUILD_BUG_ON_NULL(offsetof(struct mld2_query, mld2q_srcs) % 4),
-   BUILD_BUG_ON_NULL(offsetof(struct mld2_report, mld2r_grec) % 4),
-   BUILD_BUG_ON_NULL(offsetof(struct mld2_grec, grec_mca) % 4)
+static int __mld2_query_bugs[] __attribute__((__unused__)) = {
+   BUILD_BUG_ON_ZERO(offsetof(struct mld2_query, mld2q_srcs) % 4),
+   BUILD_BUG_ON_ZERO(offsetof(struct mld2_report, mld2r_grec) % 4),
+   BUILD_BUG_ON_ZERO(offsetof(struct mld2_grec, grec_mca) % 4)
 };
 
 static struct in6_addr mld2_all_mcr = MLD2_ALL_MCR_INIT;
-- 
2.7.4



Re: [PATCH net-next v8 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

2017-12-26 Thread Masahiro Yamada
2017-12-25 10:10 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> DT bindings for the AVE ethernet controller found on Socionext's
> UniPhier platforms.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> Acked-by: Rob Herring <r...@kernel.org>
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt   | 47 
> ++
>  1 file changed, 47 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
> b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> new file mode 100644
> index 000..8b03668
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> @@ -0,0 +1,47 @@
> +* Socionext AVE ethernet controller
> +
> +This describes the devicetree bindings for AVE ethernet controller
> +implemented on Socionext UniPhier SoCs.
> +
> +Required properties:
> + - compatible: Should be
> +   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> +   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> +   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
> +   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> + - reg: Address where registers are mapped and size of region.
> + - interrupts: Should contain the MAC interrupt.
> + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> +   "rgmii", "rmii", or "mii" according to the PHY.
> + - phy-handle: Should point to the external phy device.
> +   See ethernet.txt file in the same directory.
> + - clocks: A phandle to the clock for the MAC.
> +
> +Optional properties:
> + - resets: A phandle to the reset control for the MAC.
> + - local-mac-address: See ethernet.txt in the same directory.
> +
> +Required subnode:
> + - mdio: A container for child nodes representing phy nodes.
> + See phy.txt in the same directory.
> +
> +Example:
> +
> +   ether: ethernet@6500 {
> +   compatible = "socionext,uniphier-ld20-ave4";
> +   reg = <0x6500 0x8500>;
> +   interrupts = <0 66 4>;
> +   phy-mode = "rgmii";
> +   phy-handle = <>;
> +   clocks = <_clk 6>;
> +   resets = <_rst 6>;
> +   local-mac-address = [00 00 00 00 00 00];
> +
> +   mdio {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   ethphy: ethphy@1 {
> +   reg = <1>;
> +   };
> +   };

Andrew Lunn suggested to put a blank line before the "mdio" subnode in v7:
https://patchwork.kernel.org/patch/10127461/

Does it apply to the "ethphy" subnode, too?



Looks like you have a chance for v9.  Please consider it.


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 2/2] kbuild: remove all dummy assignments to obj-

2017-11-17 Thread Masahiro Yamada
2017-11-08 1:31 GMT+09:00 Masahiro Yamada <yamada.masah...@socionext.com>:
> Now kbuild core scripts create empty built-in.o where necessary.
> Remove "obj- := dummy.o" tricks.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>

Applied to linux-kbuild/kbuild.

-- 
Best Regards
Masahiro Yamada


[PATCH 2/2] kbuild: remove all dummy assignments to obj-

2017-11-07 Thread Masahiro Yamada
Now kbuild core scripts create empty built-in.o where necessary.
Remove "obj- := dummy.o" tricks.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 arch/arm/mach-uniphier/Makefile   | 1 -
 arch/mips/boot/dts/brcm/Makefile  | 3 ---
 arch/mips/boot/dts/cavium-octeon/Makefile | 3 ---
 arch/mips/boot/dts/img/Makefile   | 3 ---
 arch/mips/boot/dts/ingenic/Makefile   | 3 ---
 arch/mips/boot/dts/lantiq/Makefile| 3 ---
 arch/mips/boot/dts/mti/Makefile   | 3 ---
 arch/mips/boot/dts/netlogic/Makefile  | 3 ---
 arch/mips/boot/dts/ni/Makefile| 3 ---
 arch/mips/boot/dts/pic32/Makefile | 3 ---
 arch/mips/boot/dts/qca/Makefile   | 3 ---
 arch/mips/boot/dts/ralink/Makefile| 3 ---
 arch/mips/boot/dts/xilfpga/Makefile   | 3 ---
 firmware/Makefile | 3 ---
 samples/bpf/Makefile  | 3 ---
 samples/hidraw/Makefile   | 3 ---
 samples/seccomp/Makefile  | 3 ---
 samples/sockmap/Makefile  | 3 ---
 samples/statx/Makefile| 3 ---
 samples/uhid/Makefile | 3 ---
 20 files changed, 58 deletions(-)

diff --git a/arch/arm/mach-uniphier/Makefile b/arch/arm/mach-uniphier/Makefile
index 6bea3d3..e69de29 100644
--- a/arch/arm/mach-uniphier/Makefile
+++ b/arch/arm/mach-uniphier/Makefile
@@ -1 +0,0 @@
-obj- += dummy.o
diff --git a/arch/mips/boot/dts/brcm/Makefile b/arch/mips/boot/dts/brcm/Makefile
index bacb131..fcf68a2 100644
--- a/arch/mips/boot/dts/brcm/Makefile
+++ b/arch/mips/boot/dts/brcm/Makefile
@@ -34,6 +34,3 @@ dtb-$(CONFIG_DT_NONE) += \
bcm97435svmb.dtb
 
 obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/cavium-octeon/Makefile 
b/arch/mips/boot/dts/cavium-octeon/Makefile
index e9592a9..a857b4c 100644
--- a/arch/mips/boot/dts/cavium-octeon/Makefile
+++ b/arch/mips/boot/dts/cavium-octeon/Makefile
@@ -1,6 +1,3 @@
 dtb-$(CONFIG_CAVIUM_OCTEON_SOC)+= octeon_3xxx.dtb octeon_68xx.dtb
 
 obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/img/Makefile b/arch/mips/boot/dts/img/Makefile
index a46d773..17dedb7 100644
--- a/arch/mips/boot/dts/img/Makefile
+++ b/arch/mips/boot/dts/img/Makefile
@@ -2,6 +2,3 @@ dtb-$(CONFIG_FIT_IMAGE_FDT_BOSTON)  += boston.dtb
 
 dtb-$(CONFIG_MACH_PISTACHIO)   += pistachio_marduk.dtb
 obj-$(CONFIG_MACH_PISTACHIO)   += pistachio_marduk.dtb.o
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/ingenic/Makefile 
b/arch/mips/boot/dts/ingenic/Makefile
index ddd0faf..f2e516c 100644
--- a/arch/mips/boot/dts/ingenic/Makefile
+++ b/arch/mips/boot/dts/ingenic/Makefile
@@ -2,6 +2,3 @@ dtb-$(CONFIG_JZ4740_QI_LB60)+= qi_lb60.dtb
 dtb-$(CONFIG_JZ4780_CI20)  += ci20.dtb
 
 obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/lantiq/Makefile 
b/arch/mips/boot/dts/lantiq/Makefile
index 586b1c9..fed59e0 100644
--- a/arch/mips/boot/dts/lantiq/Makefile
+++ b/arch/mips/boot/dts/lantiq/Makefile
@@ -1,6 +1,3 @@
 dtb-$(CONFIG_DT_EASY50712) += easy50712.dtb
 
 obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/mti/Makefile b/arch/mips/boot/dts/mti/Makefile
index faf7ac4..35cf12b 100644
--- a/arch/mips/boot/dts/mti/Makefile
+++ b/arch/mips/boot/dts/mti/Makefile
@@ -2,6 +2,3 @@ dtb-$(CONFIG_MIPS_MALTA)+= malta.dtb
 dtb-$(CONFIG_LEGACY_BOARD_SEAD3)   += sead3.dtb
 
 obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/netlogic/Makefile 
b/arch/mips/boot/dts/netlogic/Makefile
index 77ffb30..84a38eb 100644
--- a/arch/mips/boot/dts/netlogic/Makefile
+++ b/arch/mips/boot/dts/netlogic/Makefile
@@ -5,6 +5,3 @@ dtb-$(CONFIG_DT_XLP_GVP)+= xlp_gvp.dtb
 dtb-$(CONFIG_DT_XLP_RVP)   += xlp_rvp.dtb
 
 obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))
-
-# Force kbuild to make empty built-in.o if necessary
-obj-   += dummy.o
diff --git a/arch/mips/boot/dts/ni/Makefile b/arch/mips/boot/dts/ni/Makefile
index 6cd9c60..9e2c9fa 100644
--- a/arch/mips/boot/dts/ni/Makefile
+++ b/arch/mips/boot/dts/ni/Makefile
@@ -1,4 +1 @@
 dtb-$(CONFIG_FIT_IMAGE_FDT_NI169445)   += 169445.dtb
-
-# Force kbuild to 

[PATCH 0/2] kbuild: remove all "obj- := dummy.o" tricks

2017-11-07 Thread Masahiro Yamada

This clean-up was prompted by Sam
when I refactored DT building:
https://patchwork.kernel.org/patch/10041881/

If you want to test this series,
apply the following 3 patches:
https://patchwork.kernel.org/patch/10037891/
https://patchwork.kernel.org/patch/10041877/
https://patchwork.kernel.org/patch/10041881/

I CCed DT forks to informs them of conflicts
with those patches Rob Herring offered to apply.
I doubt if he wants to review this series...



Masahiro Yamada (2):
  kbuild: create built-in.o automatically if parent directory wants it
  kbuild: remove all dummy assignments to obj-

 Makefile  | 2 +-
 arch/arm/mach-uniphier/Makefile   | 1 -
 arch/mips/boot/dts/brcm/Makefile  | 3 ---
 arch/mips/boot/dts/cavium-octeon/Makefile | 3 ---
 arch/mips/boot/dts/img/Makefile   | 3 ---
 arch/mips/boot/dts/ingenic/Makefile   | 3 ---
 arch/mips/boot/dts/lantiq/Makefile| 3 ---
 arch/mips/boot/dts/mti/Makefile   | 3 ---
 arch/mips/boot/dts/netlogic/Makefile  | 3 ---
 arch/mips/boot/dts/ni/Makefile| 3 ---
 arch/mips/boot/dts/pic32/Makefile | 3 ---
 arch/mips/boot/dts/qca/Makefile   | 3 ---
 arch/mips/boot/dts/ralink/Makefile| 3 ---
 arch/mips/boot/dts/xilfpga/Makefile   | 3 ---
 firmware/Makefile | 3 ---
 samples/bpf/Makefile  | 3 ---
 samples/hidraw/Makefile   | 3 ---
 samples/seccomp/Makefile  | 3 ---
 samples/sockmap/Makefile  | 3 ---
 samples/statx/Makefile| 3 ---
 samples/uhid/Makefile | 3 ---
 scripts/Makefile.build| 4 ++--
 22 files changed, 3 insertions(+), 61 deletions(-)

-- 
2.7.4



Re: [PATCH net-next v3 2/2] net: ethernet: socionext: add AVE ethernet driver

2017-10-24 Thread Masahiro Yamada
> +   u32 mdiosr;
> +
> +   /* wait until completion */
> +   while (--loop) {
> +   mdiosr = ave_r32(ndev, AVE_MDIOSR);
> +   if (!(mdiosr & AVE_MDIOSR_STS))
> +   break;
> +
> +   usleep_range(10, 20);
> +   }
> +
> +   if (!loop) {
> +   netdev_err(ndev,
> +  "failed to read from MDIO (status:0x%08x)\n",
> +  mdiosr);
> +   ret = -ETIMEDOUT;
> +   }
> +
> +   return ret;
> +}


The whole of this function can be replaced with
readl_poll_timeout() unless you stick to ave_r32.


I do not understand why you are such a big fan of
driver-specific accessors like ave_r32/ave_w32.



> +static int ave_mdiobus_read(struct mii_bus *bus, int phyid, int regnum)
> +{
> +   struct net_device *ndev = bus->priv;
> +   u32 mdioctl;
> +   int ret;
> +
> +   /* write address */
> +   ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
> +
> +   /* read request */
> +   mdioctl = ave_r32(ndev, AVE_MDIOCTR);
> +   ave_w32(ndev, AVE_MDIOCTR,
> +   (mdioctl | AVE_MDIOCTR_RREQ) & ~AVE_MDIOCTR_WREQ);
> +
> +   ret = ave_mdio_busywait(ndev);
> +   if (ret) {
> +   netdev_err(ndev, "phy-%d reg-%x read failed\n",
> +  phyid, regnum);
> +   return ret;
> +   }
> +
> +   return ave_r32(ndev, AVE_MDIORDR) & GENMASK(15, 0);
> +}
> +
> +static int ave_mdiobus_write(struct mii_bus *bus,
> +int phyid, int regnum, u16 val)
> +{
> +   struct net_device *ndev = bus->priv;
> +   u32 mdioctl;
> +   int ret;
> +
> +   /* write address */
> +   ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
> +
> +   /* write data */
> +   ave_w32(ndev, AVE_MDIOWDR, val);
> +
> +   /* write request */
> +   mdioctl = ave_r32(ndev, AVE_MDIOCTR);
> +   ave_w32(ndev, AVE_MDIOCTR,
> +   (mdioctl | AVE_MDIOCTR_WREQ) & ~AVE_MDIOCTR_RREQ);
> +
> +   ret = ave_mdio_busywait(ndev);
> +   if (ret)
> +   netdev_err(ndev, "phy-%d reg-%x write failed\n",
> +  phyid, regnum);
> +
> +   return ret;
> +}
> +
> +static dma_addr_t ave_dma_map(struct net_device *ndev, struct ave_desc *desc,
> + void *ptr, size_t len,
> + enum dma_data_direction dir)
> +{
> +   dma_addr_t paddr;
> +
> +   paddr = dma_map_single(ndev->dev.parent, ptr, len, dir);
> +   if (unlikely(dma_mapping_error(ndev->dev.parent, paddr))) {
> +   paddr = (dma_addr_t)-ENOMEM;

Yuk!

Re-write the code.


> +   } else {
> +   desc->skbs_dma = paddr;
> +   desc->skbs_dmalen = len;
> +   }
> +
> +   return paddr;
> +}


-- 
Best Regards
Masahiro Yamada


Re: [PATCH net-next v2 2/2] net: ethernet: socionext: add AVE ethernet driver

2017-10-18 Thread Masahiro Yamada
2017-10-18 19:23 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> On Mon, 16 Oct 2017 00:08:21 +0900 <yamada.masah...@socionext.com> wrote:

>> priv->rst = devm_reset_control_get_optional_shared(dev, NULL);
>> if (IS_ERR(priv->rst))
>>   return PTR_ERR(priv->rst);
>
> The clk and reset are optional in the driver.
> Referring to your suggested method, I'll fix the part of clk and reset.
>


Why is clk optional?





-- 
Best Regards
Masahiro Yamada


Re: [PATCH net-next v2 2/2] net: ethernet: socionext: add AVE ethernet driver

2017-10-15 Thread Masahiro Yamada
2017-10-13 9:35 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> +static int ave_probe(struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +   struct device_node *np = dev->of_node;
> +   u32 ave_id;
> +   struct ave_private *priv;
> +   const struct ave_soc_data *data;
> +   phy_interface_t phy_mode;
> +   struct net_device *ndev;
> +   struct resource *res;
> +   void __iomem *base;
> +   int irq, ret = 0;
> +   char buf[ETHTOOL_FWVERS_LEN];
> +   const void *mac_addr;
> +
> +   data = of_device_get_match_data(dev);
> +   if (WARN_ON(!data))
> +   return -EINVAL;
> +
> +   phy_mode = of_get_phy_mode(np);
> +   if (phy_mode < 0) {
> +   dev_err(dev, "phy-mode not found\n");
> +   return -EINVAL;
> +   }
> +   if ((!phy_interface_mode_is_rgmii(phy_mode)) &&
> +   phy_mode != PHY_INTERFACE_MODE_RMII &&
> +   phy_mode != PHY_INTERFACE_MODE_MII) {
> +   dev_err(dev, "phy-mode is invalid\n");
> +   return -EINVAL;
> +   }
> +
> +   irq = platform_get_irq(pdev, 0);
> +   if (irq < 0) {
> +   dev_err(dev, "IRQ not found\n");
> +   return irq;
> +   }
> +
> +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +   base = devm_ioremap_resource(dev, res);
> +   if (IS_ERR(base))
> +   return PTR_ERR(base);
> +
> +   /* alloc netdevice */
> +   ndev = alloc_etherdev(sizeof(struct ave_private));
> +   if (!ndev) {
> +   dev_err(dev, "can't allocate ethernet device\n");
> +   return -ENOMEM;
> +   }
> +
> +   ndev->netdev_ops = _netdev_ops;
> +   ndev->ethtool_ops = _ethtool_ops;
> +   SET_NETDEV_DEV(ndev, dev);
> +
> +   /* support hardware checksum */
> +   ndev->features|= (NETIF_F_IP_CSUM | NETIF_F_RXCSUM);
> +   ndev->hw_features |= (NETIF_F_IP_CSUM | NETIF_F_RXCSUM);
> +
> +   ndev->max_mtu = AVE_MAX_ETHFRAME - (ETH_HLEN + ETH_FCS_LEN);
> +
> +   /* get mac address */
> +   mac_addr = of_get_mac_address(np);
> +   if (mac_addr)
> +   ether_addr_copy(ndev->dev_addr, mac_addr);
> +
> +   /* if the mac address is invalid, use random mac address */
> +   if (!is_valid_ether_addr(ndev->dev_addr)) {
> +   eth_hw_addr_random(ndev);
> +   dev_warn(dev, "Using random MAC address: %pM\n",
> +ndev->dev_addr);
> +   }
> +
> +   priv = netdev_priv(ndev);
> +   priv->base = base;
> +   priv->irq = irq;
> +   priv->ndev = ndev;
> +   priv->msg_enable = netif_msg_init(-1, AVE_DEFAULT_MSG_ENABLE);
> +   priv->phy_mode = phy_mode;
> +   priv->data = data;
> +
> +   if (IS_DESC_64BIT(priv)) {
> +   priv->desc_size = AVE_DESC_SIZE_64;
> +   priv->tx.daddr  = AVE_TXDM_64;
> +   priv->rx.daddr  = AVE_RXDM_64;
> +   } else {
> +   priv->desc_size = AVE_DESC_SIZE_32;
> +   priv->tx.daddr  = AVE_TXDM_32;
> +   priv->rx.daddr  = AVE_RXDM_32;
> +   }
> +   priv->tx.ndesc = AVE_NR_TXDESC;
> +   priv->rx.ndesc = AVE_NR_RXDESC;
> +
> +   u64_stats_init(>stats_rx.syncp);
> +   u64_stats_init(>stats_tx.syncp);
> +
> +   /* get clock */

Please remove this super-obvious comment.


> +   priv->clk = clk_get(dev, NULL);

Missing clk_put() in the failure path.

Why don't you use devm?



> +   if (IS_ERR(priv->clk))
> +   priv->clk = NULL;

So, clk is optional, but
you need to check EPROBE_DEFER.




> +   /* get reset */

Remove.


> +   priv->rst = reset_control_get(dev, NULL);
> +   if (IS_ERR(priv->rst))
> +   priv->rst = NULL;


reset_control_get() is deprecated.  Do not use it in a new driver.

Again, missing reset_control_put().   devm?


The reset seems optional (again, ignoring EPROBE_DEFER)
but you did not use reset_control_get_optional, why?

>From your code, this reset is used as shared.


priv->rst = devm_reset_control_get_optional_shared(dev, NULL);
if (IS_ERR(priv->rst))
  return PTR_ERR(priv->rst);








-- 
Best Regards
Masahiro Yamada


Re: [PATCH net-next v2 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

2017-10-15 Thread Masahiro Yamada
2017-10-13 9:35 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> DT bindings for the AVE ethernet controller found on Socionext's
> UniPhier platforms.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt   | 53 
> ++
>  1 file changed, 53 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
> b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> new file mode 100644
> index 000..25f4d92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> @@ -0,0 +1,53 @@
> +* Socionext AVE ethernet controller
> +
> +This describes the devicetree bindings for AVE ethernet controller
> +implemented on Socionext UniPhier SoCs.
> +
> +Required properties:
> + - compatible: Should be
> +   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> +   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> +   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> +   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
> + - reg: Address where registers are mapped and size of region.
> + - interrupts: Should contain the MAC interrupt.
> + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> +   "rgmii", "rmii", or "mii" according to the PHY.
> + - pinctrl-names: List of assigned state names, see pinctrl
> +   binding documentation.
> + - pinctrl-0: List of phandles to configure the GPIO pin,
> +   see pinctrl binding documentation. Choose this appropriately
> +   according to phy-mode.
> +   - <_ether_rgmii> if phy-mode is "rgmii".
> +   - <_ether_rmii> if phy-mode is "rmii".
> +   - <_ether_mii> if phy-mode is "mii".
> + - phy-handle: Should point to the external phy device.
> +   See ethernet.txt file in the same directory.
> + - mdio subnode: Should be device tree subnode with the following required
> +   properties:
> +   - #address-cells: Must be <1>.
> +   - #size-cells: Must be <0>.
> +   - reg: phy ID number, usually a small integer.
> +
> +Optional properties:
> + - local-mac-address: See ethernet.txt in the same directory.
> +
> +Example:
> +
> +   ether: ethernet@6500 {
> +   compatible = "socionext,uniphier-ld20-ave4";
> +   reg = <0x6500 0x8500>;
> +   interrupts = <0 66 4>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ether_rgmii>;
> +   phy-mode = "rgmii";
> +   phy-handle = <>;
> +   local-mac-address = [00 00 00 00 00 00];
> +   mdio {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   ethphy: ethphy@1 {
> +       reg = <1>;
> +   };
> +   };
> +   };
> --
> 2.7.4
>


I found the following code in 2/2.

+ /* get clock */
+ priv->clk = clk_get(dev, NULL);
+ if (IS_ERR(priv->clk))
+ priv->clk = NULL;
+
+ /* get reset */
+ priv->rst = reset_control_get(dev, NULL);
+ if (IS_ERR(priv->rst))
+ priv->rst = NULL;
+


This doc needs to describe "clocks", "resets".


-- 
Best Regards
Masahiro Yamada


Re: [PATCH net-next v2 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

2017-10-13 Thread Masahiro Yamada
2017-10-13 9:35 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> DT bindings for the AVE ethernet controller found on Socionext's
> UniPhier platforms.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt   | 53 
> ++
>  1 file changed, 53 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
> b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> new file mode 100644
> index 000..25f4d92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> @@ -0,0 +1,53 @@
> +* Socionext AVE ethernet controller
> +
> +This describes the devicetree bindings for AVE ethernet controller
> +implemented on Socionext UniPhier SoCs.
> +
> +Required properties:
> + - compatible: Should be
> +   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> +   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> +   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> +   - "socionext,uniphier-ld11-ave4" : for LD11 SoC

Nit.  LD11, LD20, in this order please.

> + - reg: Address where registers are mapped and size of region.
> + - interrupts: Should contain the MAC interrupt.
> + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> +   "rgmii", "rmii", or "mii" according to the PHY.
> + - pinctrl-names: List of assigned state names, see pinctrl
> +   binding documentation.
> + - pinctrl-0: List of phandles to configure the GPIO pin,


configure the GPIO pin?

git-grep found this phrase.


$ git grep "List of phandles to configure the GPIO"
Documentation/devicetree/bindings/net/microchip,enc28j60.txt:-
pinctrl-0: List of phandles to configure the GPIO pin used as
interrupt line,





> +   see pinctrl binding documentation. Choose this appropriately
> +   according to phy-mode.
> +   - <_ether_rgmii> if phy-mode is "rgmii".
> +   - <_ether_rmii> if phy-mode is "rmii".
> +   - <_ether_mii> if phy-mode is "mii".

pinctrl_ether_rgmii is just a label
you just happened to write in your DT file.

This information is totally unrelated to hardware specification.
It is not stable, either.



> + - phy-handle: Should point to the external phy device.
> +   See ethernet.txt file in the same directory.
> + - mdio subnode: Should be device tree subnode with the following required
> +   properties:
> +   - #address-cells: Must be <1>.
> +   - #size-cells: Must be <0>.
> +   - reg: phy ID number, usually a small integer.


Are you talking about "Required subnode" in the "Required properties"?




> +Optional properties:
> + - local-mac-address: See ethernet.txt in the same directory.
> +
> +Example:
> +
> +   ether: ethernet@6500 {
> +   compatible = "socionext,uniphier-ld20-ave4";
> +   reg = <0x6500 0x8500>;
> +   interrupts = <0 66 4>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ether_rgmii>;
> +   phy-mode = "rgmii";
> +   phy-handle = <>;
> +   local-mac-address = [00 00 00 00 00 00];
> +   mdio {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   ethphy: ethphy@1 {
> +   reg = <1>;
> +   };
> +   };
> +   };
> --
> 2.7.4
>



-- 
Best Regards
Masahiro Yamada


Re: [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

2017-10-10 Thread Masahiro Yamada
2017-10-10 21:18 GMT+09:00 Matthew Wilcox <wi...@infradead.org>:
> On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
>> Reducing the header dependency will help for speeding the kernel
>> build, suppressing unnecessary recompile of objects during
>> git-bisect'ing, etc.
>
> Well, does it?  You could provide measurements showing before/after
> time to compile, or time to recompile after touching a header file that
> is included by radix-tree.h and not by radix-tree-root.h.
>
> Look at the files included (never mind the transitively included files):
>
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
>
> These are not exactly rare files to be included.  My guess is that most
> of the files in the kernel end up depending on these files *anyway*, either
> directly or through some path that isn't the radix tree.


Good question.

I tested this series, and I confirmed
the total number of included headers decreased,
but did not decrease as much as I had expected.

The statement "most of the files in the kernel end
up depending on these files" is true.

But, with that excuse,
I do not want to conclude this kind of refactoring is pointless.


For example, how can we explain
commit bc6245e5efd70c41eaf9334b1b5e646745cb0fb3 ?


 includes the following three.

#include 
#include 
#include 

Your statement applies to them too.
Actually, I did not see any impact
by replacing  in my files with 



Generally, people do not pay much attention
in decreasing header dependency.

One refactoring alone does not produce much benefits,
but making continuous efforts will disentangle the knotted threads.
Of course, this might be a pipe dream...



-- 
Best Regards
Masahiro Yamada


Re: [PATCH 10/12] net/mlx4: replace with

2017-10-09 Thread Masahiro Yamada
2017-10-09 2:32 GMT+09:00 Joe Perches <j...@perches.com>:
> On Mon, 2017-10-09 at 02:29 +0900, Masahiro Yamada wrote:
>> The idea is simple; include necessary headers explicitly.
>
> Try that for kernel.h
>
> There's a reason aggregation of #includes is useful.
>

BTW, talking about , it is too much aggregation, isn't it?

It exceed 850 lines, and contains lots of unrelated stuff in one header.
Perhaps, it could be a good time to think about splitting?

For example, I see many trace_... things in it.

I wonder if linux/kernel.h is a good home for them, or a separate file.


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

2017-10-09 Thread Masahiro Yamada
2017-10-09 3:52 GMT+09:00 Leon Romanovsky <leo...@mellanox.com>:
> On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
>
> <...>
>>
>> By splitting out the radix_tree_root definition,
>> we can reduce the header file dependency.
>>
>> Reducing the header dependency will help for speeding the kernel
>> build, suppressing unnecessary recompile of objects during
>> git-bisect'ing, etc.
>
> If we judge by the diffstat of this series, there won't be any
> visible change in anything mentioned above.


Of course, judging by the diffstat is wrong.



-- 
Best Regards
Masahiro Yamada


Re: [PATCH 10/12] net/mlx4: replace with

2017-10-08 Thread Masahiro Yamada
2017-10-09 3:55 GMT+09:00 Leon Romanovsky <l...@kernel.org>:
> On Mon, Oct 09, 2017 at 02:29:15AM +0900, Masahiro Yamada wrote:
>> 2017-10-09 2:00 GMT+09:00 David Miller <da...@davemloft.net>:
>> > From: Masahiro Yamada <yamada.masah...@socionext.com>
>> > Date: Mon,  9 Oct 2017 01:10:11 +0900
>> >
>> >> The headers
>> >>  - include/linux/mlx4/device.h
>> >>  - drivers/net/ethernet/mellanox/mlx4/mlx4.h
>> >> require the definition of struct radix_tree_root, but do not need to
>> >> know anything about other radix tree stuff.
>> >>
>> >> Include  instead of  to
>> >> reduce the header dependency.
>> >>
>> >> While we are here, let's add missing  where
>> >> radix tree accessors are used.
>> >>
>> >> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>> >
>> > Honestly this makes things more complicated.
>>
>>
>> The idea is simple; include necessary headers explicitly.
>>
>> Putting everything into one common header
>> means most of C files are forced to parse unnecessary headers.
>
> It is neglected, only first caller will actually parse that header file,
> other callers will check the #ifndef pragma without need to reparse the
> whole file.
>


You completely neglected the point of the discussion.

I am trying to not include unnecessary headers at all.




-- 
Best Regards
Masahiro Yamada


Re: [PATCH 10/12] net/mlx4: replace with

2017-10-08 Thread Masahiro Yamada
2017-10-09 2:32 GMT+09:00 Joe Perches <j...@perches.com>:
> On Mon, 2017-10-09 at 02:29 +0900, Masahiro Yamada wrote:
>> The idea is simple; include necessary headers explicitly.
>
> Try that for kernel.h
>
> There's a reason aggregation of #includes is useful.
>


We should use a common sense for the balance between
  - aggregation of #includes
  - reduce the number of parsed headers


As I had already said, if you do not like this change, you can just
throw it away.

That's fine because the impact is very limited.
Honestly, I am not so interested in this driver.
I changed sh,  net/mlx4, net/mlx5, drm/i915 for bonus while I am here.


I assume your objection is addressed to this driver change,
not the whole series.


I am more interested in the global headers like
include/linux/{irqdomain.h, fs.h} etc.


radix-tree-root.h vs radix-tree.h
has a big difference in terms of parsed headers (17 vs 128).

For example, the following is the analysis on arm64.



 include the following 17 headers:

  include/linux/radix-tree-root.h \
  include/linux/types.h \
  include/uapi/linux/types.h \
  arch/arm64/include/generated/uapi/asm/types.h \
  include/uapi/asm-generic/types.h \
  include/asm-generic/int-ll64.h \
  include/uapi/asm-generic/int-ll64.h \
  arch/arm64/include/uapi/asm/bitsperlong.h \
  include/asm-generic/bitsperlong.h \
  include/uapi/asm-generic/bitsperlong.h \
  include/uapi/linux/posix_types.h \
  include/linux/stddef.h \
  include/uapi/linux/stddef.h \
  include/linux/compiler.h \
  include/linux/compiler-gcc.h \
  arch/arm64/include/uapi/asm/posix_types.h \
  include/uapi/asm-generic/posix_types.h \




 include the following 128 headers:

  include/linux/radix-tree.h \
  include/linux/bitops.h \
  arch/arm64/include/generated/uapi/asm/types.h \
  include/uapi/asm-generic/types.h \
  include/asm-generic/int-ll64.h \
  include/uapi/asm-generic/int-ll64.h \
  arch/arm64/include/uapi/asm/bitsperlong.h \
  include/asm-generic/bitsperlong.h \
  include/uapi/asm-generic/bitsperlong.h \
  arch/arm64/include/asm/bitops.h \
  include/linux/compiler.h \
  include/linux/compiler-gcc.h \
  include/uapi/linux/types.h \
  include/uapi/linux/posix_types.h \
  include/linux/stddef.h \
  include/uapi/linux/stddef.h \
  arch/arm64/include/uapi/asm/posix_types.h \
  include/uapi/asm-generic/posix_types.h \
  arch/arm64/include/asm/barrier.h \
  include/asm-generic/barrier.h \
  include/asm-generic/bitops/builtin-__ffs.h \
  include/asm-generic/bitops/builtin-ffs.h \
  include/asm-generic/bitops/builtin-__fls.h \
  include/asm-generic/bitops/builtin-fls.h \
  include/asm-generic/bitops/ffz.h \
  include/asm-generic/bitops/fls64.h \
  include/asm-generic/bitops/find.h \
  include/asm-generic/bitops/sched.h \
  include/asm-generic/bitops/hweight.h \
  include/asm-generic/bitops/arch_hweight.h \
  include/asm-generic/bitops/const_hweight.h \
  include/asm-generic/bitops/lock.h \
  include/asm-generic/bitops/non-atomic.h \
  include/asm-generic/bitops/le.h \
  arch/arm64/include/uapi/asm/byteorder.h \
  include/linux/byteorder/big_endian.h \
  include/uapi/linux/byteorder/big_endian.h \
  include/linux/types.h \
  include/linux/swab.h \
  include/uapi/linux/swab.h \
  arch/arm64/include/generated/uapi/asm/swab.h \
  include/uapi/asm-generic/swab.h \
  include/linux/byteorder/generic.h \
  include/linux/bug.h \
  arch/arm64/include/asm/bug.h \
  include/linux/stringify.h \
  arch/arm64/include/asm/asm-bug.h \
  arch/arm64/include/asm/brk-imm.h \
  include/asm-generic/bug.h \
  include/linux/kernel.h \
  
/home/masahiro/toolchains/aarch64-linaro-4.9/gcc-linaro-4.9-2016.02-x86_64_aarch64-linux-gnu/lib/gcc/aarch64-linux-gnu/4.9.4/include/stdarg.h
\
  include/linux/linkage.h \
  include/linux/export.h \
  arch/arm64/include/asm/linkage.h \
  include/linux/log2.h \
  include/linux/typecheck.h \
  include/linux/printk.h \
  include/linux/init.h \
  include/linux/kern_levels.h \
  include/linux/cache.h \
  include/uapi/linux/kernel.h \
  include/uapi/linux/sysinfo.h \
  arch/arm64/include/asm/cache.h \
  arch/arm64/include/asm/cputype.h \
  arch/arm64/include/asm/sysreg.h \
  include/linux/dynamic_debug.h \
  include/linux/jump_label.h \
  arch/arm64/include/asm/jump_label.h \
  arch/arm64/include/asm/insn.h \
  include/linux/build_bug.h \
  include/linux/list.h \
  include/linux/poison.h \
  include/uapi/linux/const.h \
  include/linux/preempt.h \
  arch/arm64/include/generated/asm/preempt.h \
  include/asm-generic/preempt.h \
  include/linux/thread_info.h \
  include/linux/restart_block.h \
  arch/arm64/include/asm/current.h \
  arch/arm64/include/asm/thread_info.h \
  arch/arm64/include/asm/memory.h \
  arch/arm64/include/asm/page-def.h \
  arch/arm64/include/generated/asm/sizes.h \
  include/asm-generic/sizes.h \
  include/linux/sizes.h \
  include/linux/mmdebug.h \
  include/asm-generic/memory_model.h \
  include/linux/pfn.h \
  arch/arm64/include/asm/stack_pointer.h \
  include/linux/radix

Re: [PATCH 10/12] net/mlx4: replace with

2017-10-08 Thread Masahiro Yamada
2017-10-09 2:00 GMT+09:00 David Miller <da...@davemloft.net>:
> From: Masahiro Yamada <yamada.masah...@socionext.com>
> Date: Mon,  9 Oct 2017 01:10:11 +0900
>
>> The headers
>>  - include/linux/mlx4/device.h
>>  - drivers/net/ethernet/mellanox/mlx4/mlx4.h
>> require the definition of struct radix_tree_root, but do not need to
>> know anything about other radix tree stuff.
>>
>> Include  instead of  to
>> reduce the header dependency.
>>
>> While we are here, let's add missing  where
>> radix tree accessors are used.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>
> Honestly this makes things more complicated.


The idea is simple; include necessary headers explicitly.

Putting everything into one common header
means most of C files are forced to parse unnecessary headers.



> The driver was trying to consolidate all of the header needs
> by including them all in one place, the main driver header.
>
> Now you're including headers in several different files.
>
> I really don't like the results of this change and would
> ask you to reconsider.
>
> Just add both radix-tree-root.h _and_ radix-tree.h to mlx4.h
> and leave the rest of the driver alone.


If you do not like this, you can just throw it away.

 includes .
You do not need to include both.




-- 
Best Regards
Masahiro Yamada


[PATCH 11/12] net/mlx5: replace with

2017-10-08 Thread Masahiro Yamada
The header include/linux/mlx5/driver.h requires the definition of
struct radix_tree_root, but does not need to know anything about
other radix tree stuff.

Include  instead of  to
reduce the number of included header files.

Also, add  to include/linux/mlx5/gp.h where radix
tree accessors are used.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 include/linux/mlx5/driver.h | 2 +-
 include/linux/mlx5/qp.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 401c897..0aea568 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -40,7 +40,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/include/linux/mlx5/qp.h b/include/linux/mlx5/qp.h
index 66d19b6..a90996f 100644
--- a/include/linux/mlx5/qp.h
+++ b/include/linux/mlx5/qp.h
@@ -35,6 +35,7 @@
 
 #include 
 #include 
+#include 
 
 #define MLX5_INVALID_LKEY  0x100
 #define MLX5_SIG_WQE_SIZE  (MLX5_SEND_WQE_BB * 5)
-- 
2.7.4



[PATCH 00/12] radix-tree: split out struct radix_tree_root out to

2017-10-08 Thread Masahiro Yamada

The motivation of this series is to cut down unnecessary header
dependency in terms of radix tree.

Sub-systems or drivers that use radix-tree for data management
typically embed struct radix_tree_root in their data structures,
like this:

struct foo {
   ...

   struct radix_tree_root   foo_tree;
   ...
};

So,  needs to include ,
therefore, users of  include a lot of bloat
from .

If you see the definition of radix_tree_root,

   struct radix_tree_root {
   gfp_tgfp_mask;
   struct radix_tree_node   __rcu *rnode;
   };

it is a very simple structure.
It only depends on  for gfp_t and
 for __rcu.

By splitting out the radix_tree_root definition,
we can reduce the header file dependency.

Reducing the header dependency will help for speeding the kernel
build, suppressing unnecessary recompile of objects during
git-bisect'ing, etc.

The patch 1 is a trivial clean-up; it is just here
to avoid conflict.

The patch 2 is the main part of this series;
split out struct radix_tree_root.

The rest of the series replace 
with  where appropriate.

Please review if the idea is OK.

If it is OK, I'd like to know how to apply the series.

Perhaps, the first two for v4.15.  Then, rest of series
will be sent per-subsystem for v4.16?

Or, can somebody take care of the whole series?

I checked allmodconfig for x86 and arm64.
I am expecting 0 day testing will check it too.



Masahiro Yamada (12):
  radix-tree: replace  with 
  radix-tree: split struct radix_tree_root to 
  irqdomain: replace  with 
  writeback: replace  with 
  iocontext.h: replace  with

  fs: replace  with 
  blkcg: replace  with 
  fscache: include 
  sh: intc: replace  with 
  net/mlx4: replace  with 
  net/mlx5: replace  with 
  drm/i915: replace  with 

 drivers/gpu/drm/i915/i915_gem.c|  1 +
 drivers/gpu/drm/i915/i915_gem_context.c|  1 +
 drivers/gpu/drm/i915/i915_gem_context.h|  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  1 +
 drivers/gpu/drm/i915/i915_gem_object.h |  1 +
 drivers/net/ethernet/mellanox/mlx4/cq.c|  1 +
 drivers/net/ethernet/mellanox/mlx4/mlx4.h  |  2 +-
 drivers/net/ethernet/mellanox/mlx4/qp.c|  1 +
 drivers/net/ethernet/mellanox/mlx4/srq.c   |  1 +
 drivers/sh/intc/internals.h|  2 +-
 include/linux/backing-dev-defs.h   |  2 +-
 include/linux/blk-cgroup.h |  2 +-
 include/linux/fs.h |  2 +-
 include/linux/fscache.h|  1 +
 include/linux/iocontext.h  |  2 +-
 include/linux/irqdomain.h  |  2 +-
 include/linux/mlx4/device.h|  2 +-
 include/linux/mlx4/qp.h|  1 +
 include/linux/mlx5/driver.h|  2 +-
 include/linux/mlx5/qp.h|  1 +
 include/linux/radix-tree-root.h| 24 
 include/linux/radix-tree.h |  8 ++--
 22 files changed, 46 insertions(+), 16 deletions(-)
 create mode 100644 include/linux/radix-tree-root.h

-- 
2.7.4



[PATCH 10/12] net/mlx4: replace with

2017-10-08 Thread Masahiro Yamada
The headers
 - include/linux/mlx4/device.h
 - drivers/net/ethernet/mellanox/mlx4/mlx4.h
require the definition of struct radix_tree_root, but do not need to
know anything about other radix tree stuff.

Include  instead of  to
reduce the header dependency.

While we are here, let's add missing  where
radix tree accessors are used.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/net/ethernet/mellanox/mlx4/cq.c   | 1 +
 drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +-
 drivers/net/ethernet/mellanox/mlx4/qp.c   | 1 +
 drivers/net/ethernet/mellanox/mlx4/srq.c  | 1 +
 include/linux/mlx4/device.h   | 2 +-
 include/linux/mlx4/qp.h   | 1 +
 6 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c 
b/drivers/net/ethernet/mellanox/mlx4/cq.c
index 72eb50c..4cbe65c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
@@ -36,6 +36,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h 
b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index c68da19..975ef70 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -38,7 +38,7 @@
 #define MLX4_H
 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c 
b/drivers/net/ethernet/mellanox/mlx4/qp.c
index 728a2fb..50cbc62 100644
--- a/drivers/net/ethernet/mellanox/mlx4/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
@@ -35,6 +35,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/drivers/net/ethernet/mellanox/mlx4/srq.c 
b/drivers/net/ethernet/mellanox/mlx4/srq.c
index bedf521..4201a46 100644
--- a/drivers/net/ethernet/mellanox/mlx4/srq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/srq.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mlx4.h"
 #include "icm.h"
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index b0a57e0..75eac23 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -36,7 +36,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
index 8e2828d..dfa7d8e 100644
--- a/include/linux/mlx4/qp.h
+++ b/include/linux/mlx4/qp.h
@@ -35,6 +35,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 
-- 
2.7.4



Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

2017-09-08 Thread Masahiro Yamada
2017-09-08 22:02 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:

> diff --git a/drivers/net/ethernet/socionext/Kconfig 
> b/drivers/net/ethernet/socionext/Kconfig
> new file mode 100644
> index 000..788f26f
> --- /dev/null
> +++ b/drivers/net/ethernet/socionext/Kconfig
> @@ -0,0 +1,22 @@
> +config NET_VENDOR_SOCIONEXT
> +   bool "Socionext ethernet drivers"
> +   default y
> +   ---help---
> + Option to select ethernet drivers for Socionext platforms.
> +
> + Note that the answer to this question doesn't directly affect the
> + kernel: saying N will just cause the configurator to skip all
> + the questions about Agere devices. If you say Y, you will be asked
> + for your specific card in the following questions.


Agere?



> +
> +   dev_info(dev, "Socionext %c%c%c%c Ethernet IP %s (irq=%d, phy=%s)\n",
> +(ave_id >> 24) & 0xff, (ave_id >> 16) & 0xff,
> +(ave_id >> 8) & 0xff, (ave_id >> 0) & 0xff,
> +buf, ndev->irq, phy_modes(phy_mode));
> +
> +   return 0;
> +err_netdev_register:

Maybe, a bad label name.
for ex. out_del_napi or whatever.

Documentation/process/coding-style.rst says
"Choose label names which say what the goto does ..."


> +   netif_napi_del(>napi_rx);
> +   netif_napi_del(>napi_tx);
> +   mdiobus_unregister(priv->mdio);
> +err_mdiobus_register:
> +err_mdiobus_alloc:
> +err_req_irq:

These three should be merged, for ex.
out_free_device.



I ran sparse for you.

Please take a look if it is worthwhile.
Seems endianess around mac_addr.


drivers/net/ethernet/socionext/sni_ave.c:423:15: warning: cast to
restricted __le32
drivers/net/ethernet/socionext/sni_ave.c:425:20: warning: cast to
restricted __le16
drivers/net/ethernet/socionext/sni_ave.c:1194:15: warning: cast to
restricted __le32
drivers/net/ethernet/socionext/sni_ave.c:1196:20: warning: cast to
restricted __le16
drivers/net/ethernet/socionext/sni_ave.c:1398:15: warning: cast to
restricted __le32
drivers/net/ethernet/socionext/sni_ave.c:1400:20: warning: cast to
restricted __le16





-- 
Best Regards
Masahiro Yamada


Re: [PATCH 1/4] kconfig: introduce the "imply" keyword

2016-10-20 Thread Masahiro Yamada
2016-10-20 8:42 GMT+09:00 Nicolas Pitre <nicolas.pi...@linaro.org>:
> diff --git a/Documentation/kbuild/kconfig-language.txt 
> b/Documentation/kbuild/kconfig-language.txt
> index 069fcb3eef..c96127f648 100644
> --- a/Documentation/kbuild/kconfig-language.txt
> +++ b/Documentation/kbuild/kconfig-language.txt
> @@ -113,6 +113,33 @@ applicable everywhere (see syntax).
> That will limit the usefulness but on the other hand avoid
> the illegal configurations all over.
>
> +- weak reverse dependencies: "imply"  ["if" ]
> +  This is similar to "select" as it enforces a lower limit on another
> +  symbol except that the "implied" config symbol's value may still be
> +  set to n from a direct dependency or with a visible prompt.
> +  Given the following example:
> +
> +  config FOO
> +   tristate
> +   imply BAZ
> +
> +  config BAZ
> +   tristate
> +   depends on BAr


s/BAr/BAR/  ?





-- 
Best Regards
Masahiro Yamada


[PATCH 2/3] ath10k: use devm_reset_control_get() instead of reset_control_get()

2016-09-06 Thread Masahiro Yamada
Use the managed variant of reset_control_get() to simplify the
failure path and the .remove callback.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/net/wireless/ath/ath10k/ahb.c | 56 +++
 1 file changed, 10 insertions(+), 46 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ahb.c 
b/drivers/net/wireless/ath/ath10k/ahb.c
index 8a55c0c..6b355ee 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -191,92 +191,56 @@ static int ath10k_ahb_rst_ctrl_init(struct ath10k *ar)
 {
struct ath10k_ahb *ar_ahb = ath10k_ahb_priv(ar);
struct device *dev;
-   int ret;
 
dev = _ahb->pdev->dev;
 
-   ar_ahb->core_cold_rst = reset_control_get(dev, "wifi_core_cold");
+   ar_ahb->core_cold_rst = devm_reset_control_get(dev, "wifi_core_cold");
if (IS_ERR_OR_NULL(ar_ahb->core_cold_rst)) {
ath10k_err(ar, "failed to get core cold rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->core_cold_rst));
-   ret = ar_ahb->core_cold_rst ?
+   return ar_ahb->core_cold_rst ?
PTR_ERR(ar_ahb->core_cold_rst) : -ENODEV;
-   goto out;
}
 
-   ar_ahb->radio_cold_rst = reset_control_get(dev, "wifi_radio_cold");
+   ar_ahb->radio_cold_rst = devm_reset_control_get(dev, "wifi_radio_cold");
if (IS_ERR_OR_NULL(ar_ahb->radio_cold_rst)) {
ath10k_err(ar, "failed to get radio cold rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->radio_cold_rst));
-   ret = ar_ahb->radio_cold_rst ?
+   return ar_ahb->radio_cold_rst ?
PTR_ERR(ar_ahb->radio_cold_rst) : -ENODEV;
-   goto err_core_cold_rst_put;
}
 
-   ar_ahb->radio_warm_rst = reset_control_get(dev, "wifi_radio_warm");
+   ar_ahb->radio_warm_rst = devm_reset_control_get(dev, "wifi_radio_warm");
if (IS_ERR_OR_NULL(ar_ahb->radio_warm_rst)) {
ath10k_err(ar, "failed to get radio warm rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->radio_warm_rst));
-   ret = ar_ahb->radio_warm_rst ?
+   return ar_ahb->radio_warm_rst ?
PTR_ERR(ar_ahb->radio_warm_rst) : -ENODEV;
-   goto err_radio_cold_rst_put;
}
 
-   ar_ahb->radio_srif_rst = reset_control_get(dev, "wifi_radio_srif");
+   ar_ahb->radio_srif_rst = devm_reset_control_get(dev, "wifi_radio_srif");
if (IS_ERR_OR_NULL(ar_ahb->radio_srif_rst)) {
ath10k_err(ar, "failed to get radio srif rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->radio_srif_rst));
-   ret = ar_ahb->radio_srif_rst ?
+   return ar_ahb->radio_srif_rst ?
PTR_ERR(ar_ahb->radio_srif_rst) : -ENODEV;
-   goto err_radio_warm_rst_put;
}
 
-   ar_ahb->cpu_init_rst = reset_control_get(dev, "wifi_cpu_init");
+   ar_ahb->cpu_init_rst = devm_reset_control_get(dev, "wifi_cpu_init");
if (IS_ERR_OR_NULL(ar_ahb->cpu_init_rst)) {
ath10k_err(ar, "failed to get cpu init rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->cpu_init_rst));
-   ret = ar_ahb->cpu_init_rst ?
+   return ar_ahb->cpu_init_rst ?
PTR_ERR(ar_ahb->cpu_init_rst) : -ENODEV;
-   goto err_radio_srif_rst_put;
}
 
return 0;
-
-err_radio_srif_rst_put:
-   reset_control_put(ar_ahb->radio_srif_rst);
-
-err_radio_warm_rst_put:
-   reset_control_put(ar_ahb->radio_warm_rst);
-
-err_radio_cold_rst_put:
-   reset_control_put(ar_ahb->radio_cold_rst);
-
-err_core_cold_rst_put:
-   reset_control_put(ar_ahb->core_cold_rst);
-
-out:
-   return ret;
 }
 
 static void ath10k_ahb_rst_ctrl_deinit(struct ath10k *ar)
 {
struct ath10k_ahb *ar_ahb = ath10k_ahb_priv(ar);
 
-   if (!IS_ERR_OR_NULL(ar_ahb->core_cold_rst))
-   reset_control_put(ar_ahb->core_cold_rst);
-
-   if (!IS_ERR_OR_NULL(ar_ahb->radio_cold_rst))
-   reset_control_put(ar_ahb->radio_cold_rst);
-
-   if (!IS_ERR_OR_NULL(ar_ahb->radio_warm_rst))
-   reset_control_put(ar_ahb->radio_warm_rst);
-
-   if (!IS_ERR_OR_NULL(ar_ahb->radio_srif_rst))
-   reset_control_put(ar_ahb->radio_srif_rst);
-
-   if (!IS_ERR_OR_NULL(ar_ahb->cpu_init_rst))
-   reset_control_put(ar_ahb->cpu_init_rst);
-
ar_ahb->core_cold_rst = NULL;
ar_ahb->radio_cold_rst = NULL;
ar_ahb->radio_warm_rst = NULL;
-- 
1.9.1



[PATCH 3/3] ath10k: do not check if reset is NULL

2016-09-06 Thread Masahiro Yamada
Since reset_control_get() never returns NULL, we can use IS_ERR()
instead of IS_ERR_OR_NULL().  The return statements can be simpler
as well.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/net/wireless/ath/ath10k/ahb.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ahb.c 
b/drivers/net/wireless/ath/ath10k/ahb.c
index 6b355ee..b29dc4d 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -195,43 +195,38 @@ static int ath10k_ahb_rst_ctrl_init(struct ath10k *ar)
dev = _ahb->pdev->dev;
 
ar_ahb->core_cold_rst = devm_reset_control_get(dev, "wifi_core_cold");
-   if (IS_ERR_OR_NULL(ar_ahb->core_cold_rst)) {
+   if (IS_ERR(ar_ahb->core_cold_rst)) {
ath10k_err(ar, "failed to get core cold rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->core_cold_rst));
-   return ar_ahb->core_cold_rst ?
-   PTR_ERR(ar_ahb->core_cold_rst) : -ENODEV;
+   return PTR_ERR(ar_ahb->core_cold_rst);
}
 
ar_ahb->radio_cold_rst = devm_reset_control_get(dev, "wifi_radio_cold");
-   if (IS_ERR_OR_NULL(ar_ahb->radio_cold_rst)) {
+   if (IS_ERR(ar_ahb->radio_cold_rst)) {
ath10k_err(ar, "failed to get radio cold rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->radio_cold_rst));
-   return ar_ahb->radio_cold_rst ?
-   PTR_ERR(ar_ahb->radio_cold_rst) : -ENODEV;
+   return PTR_ERR(ar_ahb->radio_cold_rst);
}
 
ar_ahb->radio_warm_rst = devm_reset_control_get(dev, "wifi_radio_warm");
-   if (IS_ERR_OR_NULL(ar_ahb->radio_warm_rst)) {
+   if (IS_ERR(ar_ahb->radio_warm_rst)) {
ath10k_err(ar, "failed to get radio warm rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->radio_warm_rst));
-   return ar_ahb->radio_warm_rst ?
-   PTR_ERR(ar_ahb->radio_warm_rst) : -ENODEV;
+   return PTR_ERR(ar_ahb->radio_warm_rst);
}
 
ar_ahb->radio_srif_rst = devm_reset_control_get(dev, "wifi_radio_srif");
-   if (IS_ERR_OR_NULL(ar_ahb->radio_srif_rst)) {
+   if (IS_ERR(ar_ahb->radio_srif_rst)) {
ath10k_err(ar, "failed to get radio srif rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->radio_srif_rst));
-   return ar_ahb->radio_srif_rst ?
-   PTR_ERR(ar_ahb->radio_srif_rst) : -ENODEV;
+   return PTR_ERR(ar_ahb->radio_srif_rst);
}
 
ar_ahb->cpu_init_rst = devm_reset_control_get(dev, "wifi_cpu_init");
-   if (IS_ERR_OR_NULL(ar_ahb->cpu_init_rst)) {
+   if (IS_ERR(ar_ahb->cpu_init_rst)) {
ath10k_err(ar, "failed to get cpu init rst ctrl: %ld\n",
   PTR_ERR(ar_ahb->cpu_init_rst));
-   return ar_ahb->cpu_init_rst ?
-   PTR_ERR(ar_ahb->cpu_init_rst) : -ENODEV;
+   return PTR_ERR(ar_ahb->cpu_init_rst);
}
 
return 0;
-- 
1.9.1



[PATCH 1/3] ath10k: use devm_clk_get() instead of clk_get()

2016-09-06 Thread Masahiro Yamada
Use the managed variant of clk_get() to simplify the failure path
and the .remove callback.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/net/wireless/ath/ath10k/ahb.c | 34 ++
 1 file changed, 6 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ahb.c 
b/drivers/net/wireless/ath/ath10k/ahb.c
index acec16b..8a55c0c 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -91,59 +91,37 @@ static int ath10k_ahb_clock_init(struct ath10k *ar)
 {
struct ath10k_ahb *ar_ahb = ath10k_ahb_priv(ar);
struct device *dev;
-   int ret;
 
dev = _ahb->pdev->dev;
 
-   ar_ahb->cmd_clk = clk_get(dev, "wifi_wcss_cmd");
+   ar_ahb->cmd_clk = devm_clk_get(dev, "wifi_wcss_cmd");
if (IS_ERR_OR_NULL(ar_ahb->cmd_clk)) {
ath10k_err(ar, "failed to get cmd clk: %ld\n",
   PTR_ERR(ar_ahb->cmd_clk));
-   ret = ar_ahb->cmd_clk ? PTR_ERR(ar_ahb->cmd_clk) : -ENODEV;
-   goto out;
+   return ar_ahb->cmd_clk ? PTR_ERR(ar_ahb->cmd_clk) : -ENODEV;
}
 
-   ar_ahb->ref_clk = clk_get(dev, "wifi_wcss_ref");
+   ar_ahb->ref_clk = devm_clk_get(dev, "wifi_wcss_ref");
if (IS_ERR_OR_NULL(ar_ahb->ref_clk)) {
ath10k_err(ar, "failed to get ref clk: %ld\n",
   PTR_ERR(ar_ahb->ref_clk));
-   ret = ar_ahb->ref_clk ? PTR_ERR(ar_ahb->ref_clk) : -ENODEV;
-   goto err_cmd_clk_put;
+   return ar_ahb->ref_clk ? PTR_ERR(ar_ahb->ref_clk) : -ENODEV;
}
 
-   ar_ahb->rtc_clk = clk_get(dev, "wifi_wcss_rtc");
+   ar_ahb->rtc_clk = devm_clk_get(dev, "wifi_wcss_rtc");
if (IS_ERR_OR_NULL(ar_ahb->rtc_clk)) {
ath10k_err(ar, "failed to get rtc clk: %ld\n",
   PTR_ERR(ar_ahb->rtc_clk));
-   ret = ar_ahb->rtc_clk ? PTR_ERR(ar_ahb->rtc_clk) : -ENODEV;
-   goto err_ref_clk_put;
+   return ar_ahb->rtc_clk ? PTR_ERR(ar_ahb->rtc_clk) : -ENODEV;
}
 
return 0;
-
-err_ref_clk_put:
-   clk_put(ar_ahb->ref_clk);
-
-err_cmd_clk_put:
-   clk_put(ar_ahb->cmd_clk);
-
-out:
-   return ret;
 }
 
 static void ath10k_ahb_clock_deinit(struct ath10k *ar)
 {
struct ath10k_ahb *ar_ahb = ath10k_ahb_priv(ar);
 
-   if (!IS_ERR_OR_NULL(ar_ahb->cmd_clk))
-   clk_put(ar_ahb->cmd_clk);
-
-   if (!IS_ERR_OR_NULL(ar_ahb->ref_clk))
-   clk_put(ar_ahb->ref_clk);
-
-   if (!IS_ERR_OR_NULL(ar_ahb->rtc_clk))
-   clk_put(ar_ahb->rtc_clk);
-
ar_ahb->cmd_clk = NULL;
ar_ahb->ref_clk = NULL;
ar_ahb->rtc_clk = NULL;
-- 
1.9.1



[PATCH 0/3] ath10k: a little bit clean-up of ATH10K driver

2016-09-06 Thread Masahiro Yamada

Clean-up code with devm_clk_get() and devm_reset_control_get().

I know devm_reset_control_get() should be replaced with
either devm_reset_control_get_exclusive() or
devm_reset_control_get_shared().

My best guess is devm_reset_control_get_shared() for this case,
but I am not 100% sure, so I am leaving it to other developers.



Masahiro Yamada (3):
  ath10k: use devm_clk_get() instead of clk_get()
  ath10k: use devm_reset_control_get() instead of reset_control_get()
  ath10k: do not check if reset is NULL

 drivers/net/wireless/ath/ath10k/ahb.c | 105 +++---
 1 file changed, 21 insertions(+), 84 deletions(-)

-- 
1.9.1



[PATCH] ath10k: replace config_enabled() with IS_REACHABLE()

2016-08-23 Thread Masahiro Yamada
Commit 97f2645f358b ("tree-wide: replace config_enabled() with
IS_ENABLED()") mostly did away with config_enabled().

This is one of the postponed TODO items as config_enabled() is used
for a tristate option here.  Theoretically, config_enabled() is
equivalent to IS_BUILTIN(), but I guess IS_REACHABLE() is the best
fit for this case because both CONFIG_HWMON and CONFIG_ATH10K are
tristate.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/net/wireless/ath/ath10k/thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/thermal.c 
b/drivers/net/wireless/ath/ath10k/thermal.c
index 444b52c..0a47269 100644
--- a/drivers/net/wireless/ath/ath10k/thermal.c
+++ b/drivers/net/wireless/ath/ath10k/thermal.c
@@ -192,7 +192,7 @@ int ath10k_thermal_register(struct ath10k *ar)
 
/* Avoid linking error on devm_hwmon_device_register_with_groups, I
 * guess linux/hwmon.h is missing proper stubs. */
-   if (!config_enabled(CONFIG_HWMON))
+   if (!IS_REACHABLE(CONFIG_HWMON))
return 0;
 
hwmon_dev = devm_hwmon_device_register_with_groups(ar->dev,
-- 
1.9.1