from:"Javier Martinez Canillas"

Re: ip_auto_config() prevents network device to be registered

2017-03-17 Thread Javier Martinez Canillas

Hello,

On 01/31/2017 02:49 PM, Javier Martinez Canillas wrote:
> 
> The kernelci folks pointed out that a Samsung Exynos based board was failing
> to boot when trying to mount the rootfs via NFS, due a networking issue [0].
> 
> I looked at the issue and it turned out to be a race between ip_auto_config()
> and register_netdev() when using the ip=dhcp param in the kernel command line.
> 
> The problem is that ip_auto_config() calls wait_for_devices() [1] and returns
> as soon as it finds a network device registered. Then ic_open_devs() [2] is
> called then to bring the network devs up and wait for their carrier signals.
> 
> But ic_open_devs() grabs the rtnl_mutex lock [3] when doing this, which is the
> same lock that register_netdev() [4] grabs before registering a network 
> device.
> 
> And so if a network dev is found and wait_for_devices() returns, 
> ic_open_devs()
> will be called and no new network dev could be registered in the meantime.
> 
> So since ic_open_devs() waits up to CONF_CARRIER_TIMEOUT (120 secs) with this
> lock held, if the network dev that's supposed to get its IP over DHCP isn't 
> the
> first to be registered, the boot test job may timeout and be considered a 
> fail.
> 
> A workaround is to use ip=:eth0:dhcp instead ip=dhcp, so 
> wait_for_devices()
> waits for this specific device. Another workaround is to increase the timeout
> for the job to be much bigger than CONF_CARRIER_TIMEOUT so ip_auto_config() 
> can
> retry and the network devices can be registered between tries.
> 
> But I wonder if someone can suggest a proper way to fix this. Grabbing a mutex
> that prevents network devs to be registered for 120 secs doesn't sound 
> correct.
> 
> Thanks a lot for your help and please let me know if I misunderstood 
> something.
> 
> [0]: 
> https://storage.kernelci.org/mainline/v4.9/arm-exynos_defconfig/lab-collabora/boot-exynos5422-odroidxu3_rootfs:nfs.html
> [1]: http://lxr.free-electrons.com/source/net/ipv4/ipconfig.c#L1368
> [2]: http://lxr.free-electrons.com/source/net/ipv4/ipconfig.c#L202
> [3]: http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L68
> [4]: http://lxr.free-electrons.com/source/net/core/dev.c#L7326
> 
> 

Any comments on this?

We are still seeing this problem with today's -next (20170310):

https://storage.kernelci.org/next/next-20170310/arm-exynos_defconfig/lab-collabora/boot-exynos5422-odroidxu3.html

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

ip_auto_config() prevents network device to be registered

2017-01-31 Thread Javier Martinez Canillas

Hello,

The kernelci folks pointed out that a Samsung Exynos based board was failing
to boot when trying to mount the rootfs via NFS, due a networking issue [0].

I looked at the issue and it turned out to be a race between ip_auto_config()
and register_netdev() when using the ip=dhcp param in the kernel command line.

The problem is that ip_auto_config() calls wait_for_devices() [1] and returns
as soon as it finds a network device registered. Then ic_open_devs() [2] is
called then to bring the network devs up and wait for their carrier signals.

But ic_open_devs() grabs the rtnl_mutex lock [3] when doing this, which is the
same lock that register_netdev() [4] grabs before registering a network device.

And so if a network dev is found and wait_for_devices() returns, ic_open_devs()
will be called and no new network dev could be registered in the meantime.

So since ic_open_devs() waits up to CONF_CARRIER_TIMEOUT (120 secs) with this
lock held, if the network dev that's supposed to get its IP over DHCP isn't the
first to be registered, the boot test job may timeout and be considered a fail.

A workaround is to use ip=:eth0:dhcp instead ip=dhcp, so wait_for_devices()
waits for this specific device. Another workaround is to increase the timeout
for the job to be much bigger than CONF_CARRIER_TIMEOUT so ip_auto_config() can
retry and the network devices can be registered between tries.

But I wonder if someone can suggest a proper way to fix this. Grabbing a mutex
that prevents network devs to be registered for 120 secs doesn't sound correct.

Thanks a lot for your help and please let me know if I misunderstood something.

[0]: 
https://storage.kernelci.org/mainline/v4.9/arm-exynos_defconfig/lab-collabora/boot-exynos5422-odroidxu3_rootfs:nfs.html
[1]: http://lxr.free-electrons.com/source/net/ipv4/ipconfig.c#L1368
[2]: http://lxr.free-electrons.com/source/net/ipv4/ipconfig.c#L202
[3]: http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L68
[4]: http://lxr.free-electrons.com/source/net/core/dev.c#L7326

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH -next] net: wan: slic_ds26522: Remove .owner field for driver

2016-10-17 Thread Javier Martinez Canillas

Hello Wei,

On 10/17/2016 11:51 AM, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongj...@huawei.com>
> 
> Remove .owner field if calls are used which set it automatically.
> 
> Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
> 
> Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> ---
>  drivers/net/wan/slic_ds26522.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/wan/slic_ds26522.c b/drivers/net/wan/slic_ds26522.c
> index b776a0a..5bca31c 100644
> --- a/drivers/net/wan/slic_ds26522.c
> +++ b/drivers/net/wan/slic_ds26522.c
> @@ -241,7 +241,6 @@ static struct spi_driver slic_ds26522_driver = {
>   .driver = {
>  .name = "ds26522",
>  .bus = _bus_type,
> -.owner = THIS_MODULE,
>  .of_match_table = slic_ds26522_match,
>      },
>   .probe = slic_ds26522_probe,
> 

Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH -next] net: wan: slic_ds26522: Use module_spi_driver to simplify the code

2016-10-17 Thread Javier Martinez Canillas

Hello Wei,

On 10/17/2016 11:50 AM, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongj...@huawei.com>
> 
> module_spi_driver() makes the code simpler by eliminating
> boilerplate code.
> 
> Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> ---
>  drivers/net/wan/slic_ds26522.c | 13 +
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/net/wan/slic_ds26522.c b/drivers/net/wan/slic_ds26522.c
> index b776a0a..8f782f1 100644
> --- a/drivers/net/wan/slic_ds26522.c
> +++ b/drivers/net/wan/slic_ds26522.c
> @@ -249,15 +249,4 @@ static struct spi_driver slic_ds26522_driver = {
>   .id_table = slic_ds26522_id,
>  };
>  
> -static int __init slic_ds26522_init(void)
> -{
> - return spi_register_driver(_ds26522_driver);
> -}
> -
> -static void __exit slic_ds26522_exit(void)
> -{
> - spi_unregister_driver(_ds26522_driver);
> -}
> -
> -module_init(slic_ds26522_init);
> -module_exit(slic_ds26522_exit);
> +module_spi_driver(slic_ds26522_driver);
> 

Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH 4/7] net: qcom/emac: Fix module autoload for OF registration

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ modinfo drivers/net/ethernet/qualcomm/emac/qcom-emac.ko | grep alias
alias:  platform:qcom-emac

After this patch:

$ modinfo drivers/net/ethernet/qualcomm/emac/qcom-emac.ko | grep alias
alias:  platform:qcom-emac
alias:  of:N*T*Cqcom,fsm9900-emacC*
alias:  of:N*T*Cqcom,fsm9900-emac

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/qualcomm/emac/emac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c 
b/drivers/net/ethernet/qualcomm/emac/emac.c
index 9bf3b2b82e95..4fede4b86538 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -575,6 +575,7 @@ static const struct of_device_id emac_dt_match[] = {
},
{}
 };
+MODULE_DEVICE_TABLE(of, emac_dt_match);
 
 #if IS_ENABLED(CONFIG_ACPI)
 static const struct acpi_device_id emac_acpi_match[] = {
-- 
2.7.4

[PATCH 6/7] net: dsa: b53: Fix module autoload

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ modinfo drivers/net/dsa/b53/b53_mmap.ko  | grep alias
$

After this patch:

$ modinfo drivers/net/dsa/b53/b53_mmap.ko  | grep alias
alias:  of:N*T*Cbrcm,bcm63xx-switchC*
alias:  of:N*T*Cbrcm,bcm63xx-switch
alias:  of:N*T*Cbrcm,bcm6368-switchC*
alias:  of:N*T*Cbrcm,bcm6368-switch
alias:  of:N*T*Cbrcm,bcm6328-switchC*
alias:  of:N*T*Cbrcm,bcm6328-switch
alias:  of:N*T*Cbrcm,bcm3384-switchC*
alias:  of:N*T*Cbrcm,bcm3384-switch

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/dsa/b53/b53_mmap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/b53/b53_mmap.c b/drivers/net/dsa/b53/b53_mmap.c
index 76fb8552c9d9..ef63d24fef81 100644
--- a/drivers/net/dsa/b53/b53_mmap.c
+++ b/drivers/net/dsa/b53/b53_mmap.c
@@ -256,6 +256,7 @@ static const struct of_device_id b53_mmap_of_table[] = {
{ .compatible = "brcm,bcm63xx-switch" },
{ /* sentinel */ },
 };
+MODULE_DEVICE_TABLE(of, b53_mmap_of_table);
 
 static struct platform_driver b53_mmap_driver = {
.probe = b53_mmap_probe,
-- 
2.7.4

[PATCH 7/7] net: dsa: bcm_sf2: Fix module autoload for OF registration

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ modinfo drivers/net/dsa/bcm_sf2.ko | grep alias
alias:  platform:brcm-sf2

After this patch:

$ modinfo drivers/net/dsa/bcm_sf2.ko | grep alias
alias:  platform:brcm-sf2
alias:  of:N*T*Cbrcm,bcm7445-switch-v4.0C*
alias:  of:N*T*Cbrcm,bcm7445-switch-v4.0

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/dsa/bcm_sf2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index e218887f18b7..0427009bc924 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -1158,6 +1158,7 @@ static const struct of_device_id bcm_sf2_of_match[] = {
{ .compatible = "brcm,bcm7445-switch-v4.0" },
{ /* sentinel */ },
 };
+MODULE_DEVICE_TABLE(of, bcm_sf2_of_match);
 
 static struct platform_driver bcm_sf2_driver = {
.probe  = bcm_sf2_sw_probe,
-- 
2.7.4

[PATCH 3/7] net: hns: Fix hns_dsaf module autoload for OF registration

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ modinfo drivers/net/ethernet/hisilicon/hns/hns_dsaf.ko | grep alias
alias:  acpi*:HISI00B2:*
alias:  acpi*:HISI00B1:*

After this patch:

$ modinfo drivers/net/ethernet/hisilicon/hns/hns_dsaf.ko | grep alias
alias:  acpi*:HISI00B2:*
alias:  acpi*:HISI00B1:*
alias:  of:N*T*Chisilicon,hns-dsaf-v2C*
alias:  of:N*T*Chisilicon,hns-dsaf-v2
alias:  of:N*T*Chisilicon,hns-dsaf-v1C*
alias:  of:N*T*Chisilicon,hns-dsaf-v1

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 8d70377f6624..8ea3d95fa483 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -2751,6 +2751,7 @@ static const struct of_device_id g_dsaf_match[] = {
{.compatible = "hisilicon,hns-dsaf-v2"},
{}
 };
+MODULE_DEVICE_TABLE(of, g_dsaf_match);
 
 static struct platform_driver g_dsaf_driver = {
.probe = hns_dsaf_probe,
-- 
2.7.4

[PATCH 1/7] net: nps_enet: Fix module autoload

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ modinfo drivers/net/ethernet/ezchip/nps_enet.ko | grep alias
$

After this patch:

$ modinfo drivers/net/ethernet/ezchip/nps_enet.ko | grep alias
alias:  of:N*T*Cezchip,nps-mgt-enetC*
alias:  of:N*T*Cezchip,nps-mgt-enet

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/ezchip/nps_enet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c 
b/drivers/net/ethernet/ezchip/nps_enet.c
index f928e6f79c89..223f35cc034c 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -669,6 +669,7 @@ static const struct of_device_id nps_enet_dt_ids[] = {
{ .compatible = "ezchip,nps-mgt-enet" },
{ /* Sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, nps_enet_dt_ids);
 
 static struct platform_driver nps_enet_driver = {
.probe = nps_enet_probe,
-- 
2.7.4

[PATCH 5/7] net: hisilicon: Fix hns_mdio module autoload for OF registration

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ modinfo drivers/net/ethernet/hisilicon//hns_mdio.ko | grep alias
alias:  platform:Hi-HNS_MDIO
alias:  acpi*:HISI0141:*

After this patch:

$ modinfo drivers/net/ethernet/hisilicon//hns_mdio.ko | grep alias
alias:  platform:Hi-HNS_MDIO
alias:  of:N*T*Chisilicon,hns-mdioC*
alias:  of:N*T*Chisilicon,hns-mdio
alias:  of:N*T*Chisilicon,mdioC*
alias:  of:N*T*Chisilicon,mdio
alias:  acpi*:HISI0141:*

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/hisilicon/hns_mdio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c 
b/drivers/net/ethernet/hisilicon/hns_mdio.c
index 33f4c483af0f..501eb2090ca6 100644
--- a/drivers/net/ethernet/hisilicon/hns_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
@@ -563,6 +563,7 @@ static const struct of_device_id hns_mdio_match[] = {
{.compatible = "hisilicon,hns-mdio"},
{}
 };
+MODULE_DEVICE_TABLE(of, hns_mdio_match);
 
 static const struct acpi_device_id hns_mdio_acpi_match[] = {
{ "HISI0141", 0 },
-- 
2.7.4

[PATCH 2/7] net: ethernet: nb8800: Fix module autoload

2016-10-17 Thread Javier Martinez Canillas

If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.

Export the module alias information using the MODULE_DEVICE_TABLE() macro.

Before this patch:

$ $ modinfo drivers/net/ethernet/aurora/nb8800.ko | grep alias
$

After this patch:

$ modinfo drivers/net/ethernet/aurora/nb8800.ko | grep alias
alias:  of:N*T*Csigma,smp8734-ethernetC*
alias:  of:N*T*Csigma,smp8734-ethernet
alias:  of:N*T*Csigma,smp8642-ethernetC*
alias:  of:N*T*Csigma,smp8642-ethernet
alias:  of:N*T*Caurora,nb8800C*
alias:  of:N*T*Caurora,nb8800

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/aurora/nb8800.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/aurora/nb8800.c 
b/drivers/net/ethernet/aurora/nb8800.c
index 453dc0967125..d5f2ad1a5a30 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1357,6 +1357,7 @@ static const struct of_device_id nb8800_dt_ids[] = {
},
{ }
 };
+MODULE_DEVICE_TABLE(of, nb8800_dt_ids);
 
 static int nb8800_probe(struct platform_device *pdev)
 {
-- 
2.7.4

[PATCH 0/7] net: Fix module autoload for several platform drivers

2016-10-17 Thread Javier Martinez Canillas

Hello David,

I noticed that module autoload won't be working in a bunch of platform
drivers in the net subsystem and this patch series contains the fixes.

Best regards,
Javier


Javier Martinez Canillas (7):
  net: nps_enet: Fix module autoload
  net: ethernet: nb8800: Fix module autoload
  net: hns: Fix hns_dsaf module autoload for OF registration
  net: qcom/emac: Fix module autoload for OF registration
  net: hisilicon: Fix hns_mdio module autoload for OF registration
  net: dsa: b53: Fix module autoload
  net: dsa: bcm_sf2: Fix module autoload for OF registration

 drivers/net/dsa/b53/b53_mmap.c | 1 +
 drivers/net/dsa/bcm_sf2.c  | 1 +
 drivers/net/ethernet/aurora/nb8800.c   | 1 +
 drivers/net/ethernet/ezchip/nps_enet.c | 1 +
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 1 +
 drivers/net/ethernet/hisilicon/hns_mdio.c  | 1 +
 drivers/net/ethernet/qualcomm/emac/emac.c  | 1 +
 7 files changed, 7 insertions(+)

-- 
2.7.4

[PATCH] net: wan: slic_ds26522: Allow driver to built if COMPILE_TEST is enabled

2016-10-12 Thread Javier Martinez Canillas

The driver only has runtime but no build time dependency with FSL_SOC ||
ARCH_MXC || ARCH_LAYERSCAPE.  So it can be built for testing purposes if
the COMPILE_TEST option is enabled.

This is useful to have more build coverage and make sure that the driver
is not affected by changes that could cause build regressions.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wan/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig
index 33ab3345d333..4e9fe75d7067 100644
--- a/drivers/net/wan/Kconfig
+++ b/drivers/net/wan/Kconfig
@@ -294,7 +294,7 @@ config FSL_UCC_HDLC
 config SLIC_DS26522
tristate "Slic Maxim ds26522 card support"
depends on SPI
-   depends on FSL_SOC || ARCH_MXC || ARCH_LAYERSCAPE
+   depends on FSL_SOC || ARCH_MXC || ARCH_LAYERSCAPE || COMPILE_TEST
help
  This module initializes and configures the slic maxim card
  in T1 or E1 mode.
-- 
2.7.4

[PATCH 1/2] net: wan: slic_ds26522: add SPI device ID table to fix module autoload

2016-10-12 Thread Javier Martinez Canillas

If the driver is built as a module, module alias information isn't filled
so the module won't be autoloaded. Add a SPI device ID table and use the
MODULE_DEVICE_TABLE() macro so the information is exported in the module.

Before this patch:

$ modinfo drivers/net/wan/slic_ds26522.ko | grep alias
$

After this patch:

$ modinfo drivers/net/wan/slic_ds26522.ko | grep alias
alias:  spi:ds26522

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wan/slic_ds26522.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wan/slic_ds26522.c b/drivers/net/wan/slic_ds26522.c
index d06a887a2352..53366a2232f0 100644
--- a/drivers/net/wan/slic_ds26522.c
+++ b/drivers/net/wan/slic_ds26522.c
@@ -223,6 +223,12 @@ static int slic_ds26522_probe(struct spi_device *spi)
return ret;
 }
 
+static const struct spi_device_id slic_ds26522_id[] = {
+   { .name = "ds26522" },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(spi, slic_ds26522_id);
+
 static const struct of_device_id slic_ds26522_match[] = {
{
 .compatible = "maxim,ds26522",
@@ -239,6 +245,7 @@ static struct spi_driver slic_ds26522_driver = {
   },
.probe = slic_ds26522_probe,
.remove = slic_ds26522_remove,
+   .id_table = slic_ds26522_id,
 };
 
 static int __init slic_ds26522_init(void)
-- 
2.7.4

[PATCH 2/2] net: wan: slic_ds26522: Export OF module alias information

2016-10-12 Thread Javier Martinez Canillas

When the device is registered via OF, the OF table is used to match the
driver instead of the SPI device ID table, but the entries in the later
are used as aliasses to load the module if the driver was not built-in.

This is because the SPI core always reports an SPI module alias instead
of an OF one, but that could change so it's better to always export it.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wan/slic_ds26522.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wan/slic_ds26522.c b/drivers/net/wan/slic_ds26522.c
index 53366a2232f0..b776a0ab106c 100644
--- a/drivers/net/wan/slic_ds26522.c
+++ b/drivers/net/wan/slic_ds26522.c
@@ -235,6 +235,7 @@ static const struct of_device_id slic_ds26522_match[] = {
 },
{},
 };
+MODULE_DEVICE_TABLE(of, slic_ds26522_match);
 
 static struct spi_driver slic_ds26522_driver = {
.driver = {
-- 
2.7.4

[PATCH 12/15] sis900: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/sis/sis900.c | 4 ++--
 drivers/net/ethernet/sis/sis900.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sis/sis900.c 
b/drivers/net/ethernet/sis/sis900.c
index 95001ee408ab..6f85276376e8 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -1426,7 +1426,7 @@ static void sis900_set_mode(struct sis900_private *sp, 
int speed, int duplex)
rx_flags |= RxATX;
}
 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
/* Can accept Jumbo packet */
rx_flags |= RxAJAB;
 #endif
@@ -1750,7 +1750,7 @@ static int sis900_rx(struct net_device *net_dev)
data_size = rx_status & DSIZE;
rx_size = data_size - CRC_SIZE;
 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
/* ``TOOLONG'' flag means jumbo packet received. */
if ((rx_status & TOOLONG) && data_size <= MAX_FRAME_SIZE)
rx_status &= (~ ((unsigned int)TOOLONG));
diff --git a/drivers/net/ethernet/sis/sis900.h 
b/drivers/net/ethernet/sis/sis900.h
index 7d430d322931..f0da3dc52c01 100644
--- a/drivers/net/ethernet/sis/sis900.h
+++ b/drivers/net/ethernet/sis/sis900.h
@@ -310,7 +310,7 @@ enum sis630_revision_id {
 #define CRC_SIZE4
 #define MAC_HEADER_SIZE 14
 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
 #define MAX_FRAME_SIZE  (1518 + 4)
 #else
 #define MAX_FRAME_SIZE  1518
-- 
2.7.4

[PATCH 06/15] net/fsl_pq_mdio: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c 
b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index f3c63dce1e30..446c7b374ff5 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -195,7 +195,7 @@ static int fsl_pq_mdio_reset(struct mii_bus *bus)
return 0;
 }
 
-#if defined(CONFIG_GIANFAR) || defined(CONFIG_GIANFAR_MODULE)
+#if IS_ENABLED(CONFIG_GIANFAR)
 /*
  * Return the TBIPA address, starting from the address
  * of the mapped GFAR MDIO registers (struct gfar)
@@ -228,7 +228,7 @@ static uint32_t __iomem *get_etsec_tbipa(void __iomem *p)
 }
 #endif
 
-#if defined(CONFIG_UCC_GETH) || defined(CONFIG_UCC_GETH_MODULE)
+#if IS_ENABLED(CONFIG_UCC_GETH)
 /*
  * Return the TBIPAR address for a QE MDIO node, starting from the address
  * of the mapped MII registers (struct fsl_pq_mii)
@@ -306,7 +306,7 @@ static void ucc_configure(phys_addr_t start, phys_addr_t 
end)
 #endif
 
 static const struct of_device_id fsl_pq_mdio_match[] = {
-#if defined(CONFIG_GIANFAR) || defined(CONFIG_GIANFAR_MODULE)
+#if IS_ENABLED(CONFIG_GIANFAR)
{
.compatible = "fsl,gianfar-tbi",
.data = &(struct fsl_pq_mdio_data) {
@@ -344,7 +344,7 @@ static const struct of_device_id fsl_pq_mdio_match[] = {
},
},
 #endif
-#if defined(CONFIG_UCC_GETH) || defined(CONFIG_UCC_GETH_MODULE)
+#if IS_ENABLED(CONFIG_UCC_GETH)
{
.compatible = "fsl,ucc-mdio",
.data = &(struct fsl_pq_mdio_data) {
-- 
2.7.4

[PATCH 02/15] starfire: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/adaptec/starfire.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/adaptec/starfire.c 
b/drivers/net/ethernet/adaptec/starfire.c
index 1d1069641d81..8af2c88d5b33 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -66,7 +66,7 @@
  */
 #define ZEROCOPY
 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
 #define VLAN_SUPPORT
 #endif
 
-- 
2.7.4

[PATCH 04/15] bnx2: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/broadcom/bnx2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2.c 
b/drivers/net/ethernet/broadcom/bnx2.c
index 8fc3f3c137f8..ecd357dbb1d4 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -50,7 +50,7 @@
 #include 
 #include 
 
-#if defined(CONFIG_CNIC) || defined(CONFIG_CNIC_MODULE)
+#if IS_ENABLED(CONFIG_CNIC)
 #define BCM_CNIC 1
 #include "cnic_if.h"
 #endif
-- 
2.7.4

[PATCH 05/15] sundance: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/dlink/sundance.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/dlink/sundance.c 
b/drivers/net/ethernet/dlink/sundance.c
index 58c6338a839e..79d80090eac8 100644
--- a/drivers/net/ethernet/dlink/sundance.c
+++ b/drivers/net/ethernet/dlink/sundance.c
@@ -867,7 +867,7 @@ static int netdev_open(struct net_device *dev)
 
/* Initialize other registers. */
__set_mac_addr(dev);
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
iowrite16(dev->mtu + 18, ioaddr + MaxFrameSize);
 #else
iowrite16(dev->mtu + 14, ioaddr + MaxFrameSize);
-- 
2.7.4

[PATCH 08/15] ixgbe: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 33c025055011..b06e32d0d22a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -45,10 +45,10 @@
 #include "ixgbe_type.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb.h"
-#if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
+#if IS_ENABLED(CONFIG_FCOE)
 #define IXGBE_FCOE
 #include "ixgbe_fcoe.h"
-#endif /* CONFIG_FCOE or CONFIG_FCOE_MODULE */
+#endif /* IS_ENABLED(CONFIG_FCOE) */
 #ifdef CONFIG_IXGBE_DCA
 #include 
 #endif
-- 
2.7.4

[PATCH 03/15] ethernet: amd: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/amd/7990.c | 6 +++---
 drivers/net/ethernet/amd/amd8111e.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/amd/7990.c b/drivers/net/ethernet/amd/7990.c
index dcf2a1f3643d..dc57f2759f44 100644
--- a/drivers/net/ethernet/amd/7990.c
+++ b/drivers/net/ethernet/amd/7990.c
@@ -45,14 +45,14 @@
 #define WRITERDP(lp, x)out_be16(lp->base + LANCE_RDP, (x))
 #define READRDP(lp)in_be16(lp->base + LANCE_RDP)
 
-#if defined(CONFIG_HPLANCE) || defined(CONFIG_HPLANCE_MODULE)
+#if IS_ENABLED(CONFIG_HPLANCE)
 #include "hplance.h"
 
 #undef WRITERAP
 #undef WRITERDP
 #undef READRDP
 
-#if defined(CONFIG_MVME147_NET) || defined(CONFIG_MVME147_NET_MODULE)
+#if IS_ENABLED(CONFIG_MVME147_NET)
 
 /* Lossage Factor Nine, Mr Sulu. */
 #define WRITERAP(lp, x)(lp->writerap(lp, x))
@@ -86,7 +86,7 @@ static inline __u16 READRDP(struct lance_private *lp)
 }
 
 #endif
-#endif /* CONFIG_HPLANCE || CONFIG_HPLANCE_MODULE */
+#endif /* IS_ENABLED(CONFIG_HPLANCE) */
 
 /* debugging output macros, various flavours */
 /* #define TEST_HITS */
diff --git a/drivers/net/ethernet/amd/amd8111e.c 
b/drivers/net/ethernet/amd/amd8111e.c
index 94960055fa1f..f92cc97151ec 100644
--- a/drivers/net/ethernet/amd/amd8111e.c
+++ b/drivers/net/ethernet/amd/amd8111e.c
@@ -89,7 +89,7 @@ Revision History:
 #include 
 #include 
 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
 #define AMD8111E_VLAN_TAG_USED 1
 #else
 #define AMD8111E_VLAN_TAG_USED 0
-- 
2.7.4

[PATCH 00/15] drivers: net: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

Hello David,

This trivial series is similar to [0] for net/ that you already merged, but
for drivers/net. The patches replaces the open coding to check for a Kconfig
symbol being built-in or module, with IS_ENABLED() macro that does the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

[0]: https://lkml.org/lkml/2016/9/9/323

Best regards,
Javier


Javier Martinez Canillas (15):
  3c59x: use IS_ENABLED() instead of checking for built-in or module
  starfire: use IS_ENABLED() instead of checking for built-in or module
  ethernet: amd: use IS_ENABLED() instead of checking for built-in or
module
  bnx2: use IS_ENABLED() instead of checking for built-in or module
  sundance: use IS_ENABLED() instead of checking for built-in or module
  net/fsl_pq_mdio: use IS_ENABLED() instead of checking for built-in or
module
  i825xx: use IS_ENABLED() instead of checking for built-in or module
  ixgbe: use IS_ENABLED() instead of checking for built-in or module
  net: mvneta: use IS_ENABLED() instead of checking for built-in or
module
  natsemi: use IS_ENABLED() instead of checking for built-in or module
  sfc: use IS_ENABLED() instead of checking for built-in or module
  sis900: use IS_ENABLED() instead of checking for built-in or module
  stmmac: use IS_ENABLED() instead of checking for built-in or module
  hamradio: use IS_ENABLED() instead of checking for built-in or module
  iwlegacy: use IS_ENABLED() instead of checking for built-in or module

 drivers/net/ethernet/3com/3c59x.c| 2 +-
 drivers/net/ethernet/adaptec/starfire.c  | 2 +-
 drivers/net/ethernet/amd/7990.c  | 6 +++---
 drivers/net/ethernet/amd/amd8111e.c  | 2 +-
 drivers/net/ethernet/broadcom/bnx2.c | 2 +-
 drivers/net/ethernet/dlink/sundance.c| 2 +-
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 8 
 drivers/net/ethernet/i825xx/82596.c  | 4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
 drivers/net/ethernet/marvell/mvneta_bm.h | 2 +-
 drivers/net/ethernet/natsemi/ns83820.c   | 2 +-
 drivers/net/ethernet/sfc/falcon_boards.c | 4 ++--
 drivers/net/ethernet/sis/sis900.c| 4 ++--
 drivers/net/ethernet/sis/sis900.h| 2 +-
 drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
 drivers/net/hamradio/bpqether.c  | 2 +-
 drivers/net/wireless/intel/iwlegacy/common.h | 4 ++--
 17 files changed, 27 insertions(+), 27 deletions(-)

-- 
2.7.4

[PATCH 07/15] i825xx: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/i825xx/82596.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/i825xx/82596.c 
b/drivers/net/ethernet/i825xx/82596.c
index befb4ac3e2b0..ce235b776793 100644
--- a/drivers/net/ethernet/i825xx/82596.c
+++ b/drivers/net/ethernet/i825xx/82596.c
@@ -89,10 +89,10 @@ static char version[] __initdata =
 #define DEB(x,y)   if (i596_debug & (x)) y
 
 
-#if defined(CONFIG_MVME16x_NET) || defined(CONFIG_MVME16x_NET_MODULE)
+#if IS_ENABLED(CONFIG_MVME16x_NET)
 #define ENABLE_MVME16x_NET
 #endif
-#if defined(CONFIG_BVME6000_NET) || defined(CONFIG_BVME6000_NET_MODULE)
+#if IS_ENABLED(CONFIG_BVME6000_NET)
 #define ENABLE_BVME6000_NET
 #endif
 
-- 
2.7.4

[PATCH 09/15] net: mvneta: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/marvell/mvneta_bm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvneta_bm.h 
b/drivers/net/ethernet/marvell/mvneta_bm.h
index e74fd44a92f7..a32de432800c 100644
--- a/drivers/net/ethernet/marvell/mvneta_bm.h
+++ b/drivers/net/ethernet/marvell/mvneta_bm.h
@@ -133,7 +133,7 @@ struct mvneta_bm_pool {
 void *mvneta_frag_alloc(unsigned int frag_size);
 void mvneta_frag_free(unsigned int frag_size, void *data);
 
-#if defined(CONFIG_MVNETA_BM) || defined(CONFIG_MVNETA_BM_MODULE)
+#if IS_ENABLED(CONFIG_MVNETA_BM)
 void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
struct mvneta_bm_pool *bm_pool, u8 port_map);
 void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct mvneta_bm_pool 
*bm_pool,
-- 
2.7.4

[PATCH 11/15] sfc: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/sfc/falcon_boards.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon_boards.c 
b/drivers/net/ethernet/sfc/falcon_boards.c
index 1736f4b806af..f6883b2b5da3 100644
--- a/drivers/net/ethernet/sfc/falcon_boards.c
+++ b/drivers/net/ethernet/sfc/falcon_boards.c
@@ -64,7 +64,7 @@
 #define LM87_ALARM_TEMP_INT0x10
 #define LM87_ALARM_TEMP_EXT1   0x20
 
-#if defined(CONFIG_SENSORS_LM87) || defined(CONFIG_SENSORS_LM87_MODULE)
+#if IS_ENABLED(CONFIG_SENSORS_LM87)
 
 static int efx_poke_lm87(struct i2c_client *client, const u8 *reg_values)
 {
@@ -455,7 +455,7 @@ static int sfe4001_init(struct efx_nic *efx)
struct falcon_board *board = falcon_board(efx);
int rc;
 
-#if defined(CONFIG_SENSORS_LM90) || defined(CONFIG_SENSORS_LM90_MODULE)
+#if IS_ENABLED(CONFIG_SENSORS_LM90)
board->hwmon_client =
i2c_new_device(>i2c_adap, _hwmon_info);
 #else
-- 
2.7.4

[PATCH 13/15] stmmac: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 2533b91f1421..d3292c4a6eda 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -30,7 +30,7 @@
 #include 
 #include 
 #include 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
 #define STMMAC_VLAN_TAG_USED
 #include 
 #endif
-- 
2.7.4

[PATCH 15/15] iwlegacy: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/wireless/intel/iwlegacy/common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/common.h 
b/drivers/net/wireless/intel/iwlegacy/common.h
index 726ede391cb9..3bba521d2cd9 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.h
+++ b/drivers/net/wireless/intel/iwlegacy/common.h
@@ -1320,7 +1320,7 @@ struct il_priv {
u64 timestamp;
 
union {
-#if defined(CONFIG_IWL3945) || defined(CONFIG_IWL3945_MODULE)
+#if IS_ENABLED(CONFIG_IWL3945)
struct {
void *shared_virt;
dma_addr_t shared_phys;
@@ -1351,7 +1351,7 @@ struct il_priv {
 
} _3945;
 #endif
-#if defined(CONFIG_IWL4965) || defined(CONFIG_IWL4965_MODULE)
+#if IS_ENABLED(CONFIG_IWL4965)
struct {
struct il_rx_phy_res last_phy_res;
bool last_phy_res_valid;
-- 
2.7.4

[PATCH 14/15] hamradio: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/hamradio/bpqether.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index d95a50ae996d..622ab3ab9e93 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -484,7 +484,7 @@ static void bpq_setup(struct net_device *dev)
dev->flags  = 0;
dev->features   = NETIF_F_LLTX; /* Allow recursion */
 
-#if defined(CONFIG_AX25) || defined(CONFIG_AX25_MODULE)
+#if IS_ENABLED(CONFIG_AX25)
dev->header_ops  = _header_ops;
 #endif
 
-- 
2.7.4

[PATCH 10/15] natsemi: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/natsemi/ns83820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c 
b/drivers/net/ethernet/natsemi/ns83820.c
index eb807b0dc72a..569ade6cf85c 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -134,7 +134,7 @@ static int lnksts = 0;  /* CFG_LNKSTS bit 
polarity */
 
 /* tunables */
 #define RX_BUF_SIZE1500/* 8192 */
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
 #define NS83820_VLAN_ACCEL_SUPPORT
 #endif
 
-- 
2.7.4

[PATCH 01/15] 3c59x: use IS_ENABLED() instead of checking for built-in or module

2016-09-12 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/3com/3c59x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/3com/3c59x.c 
b/drivers/net/ethernet/3com/3c59x.c
index 25c55ab05c7d..9133e7926da5 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -3089,7 +3089,7 @@ static void set_rx_mode(struct net_device *dev)
iowrite16(new_mode, ioaddr + EL3_CMD);
 }
 
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
 /* Setup the card so that it can receive frames with an 802.1q VLAN tag.
Note that this must be done after each RxReset due to some backwards
compatibility logic in the Cyclone and Tornado ASICs */
-- 
2.7.4

[PATCH 2/8] lec: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/atm/lec.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index e574a7e9db6f..5d2693826afb 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -31,7 +31,7 @@
 #include 
 
 /* Proxy LEC knows about bridging */
-#if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
+#if IS_ENABLED(CONFIG_BRIDGE)
 #include "../bridge/br_private.h"
 
 static unsigned char bridge_ula_lec[] = { 0x01, 0x80, 0xc2, 0x00, 0x00 };
@@ -121,7 +121,7 @@ static unsigned char bus_mac[ETH_ALEN] = { 0xff, 0xff, 
0xff, 0xff, 0xff, 0xff };
 /* Device structures */
 static struct net_device *dev_lec[MAX_LEC_ITF];
 
-#if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
+#if IS_ENABLED(CONFIG_BRIDGE)
 static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
 {
char *buff;
@@ -155,7 +155,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct 
net_device *dev)
sk->sk_data_ready(sk);
}
 }
-#endif /* defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) */
+#endif /* IS_ENABLED(CONFIG_BRIDGE) */
 
 /*
  * Open/initialize the netdevice. This is called (in the current kernel)
@@ -222,7 +222,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
pr_debug("skbuff head:%lx data:%lx tail:%lx end:%lx\n",
 (long)skb->head, (long)skb->data, (long)skb_tail_pointer(skb),
 (long)skb_end_pointer(skb));
-#if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
+#if IS_ENABLED(CONFIG_BRIDGE)
if (memcmp(skb->data, bridge_ula_lec, sizeof(bridge_ula_lec)) == 0)
lec_handle_bridge(skb, dev);
 #endif
@@ -426,7 +426,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff 
*skb)
(unsigned short)(0x & mesg->content.normal.flag);
break;
case l_should_bridge:
-#if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
+#if IS_ENABLED(CONFIG_BRIDGE)
{
pr_debug("%s: bridge zeppelin asks about %pM\n",
 dev->name, mesg->content.proxy.mac_addr);
@@ -452,7 +452,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff 
*skb)
sk->sk_data_ready(sk);
}
}
-#endif /* defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) */
+#endif /* IS_ENABLED(CONFIG_BRIDGE) */
break;
default:
pr_info("%s: Unknown message type %d\n", dev->name, mesg->type);
-- 
2.7.4

[PATCH 1/8] appletalk: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/appletalk/ddp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index f066781be3c8..10d2bdce686e 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1278,7 +1278,7 @@ out:
return err;
 }
 
-#if defined(CONFIG_IPDDP) || defined(CONFIG_IPDDP_MODULE)
+#if IS_ENABLED(CONFIG_IPDDP)
 static __inline__ int is_ip_over_ddp(struct sk_buff *skb)
 {
return skb->data[12] == 22;
-- 
2.7.4

[PATCH 4/8] ipv4: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/ipv4/ip_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 65569274efb8..b913f5bf0757 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -490,7 +490,7 @@ static void ip_copy_metadata(struct sk_buff *to, struct 
sk_buff *from)
to->tc_index = from->tc_index;
 #endif
nf_copy(to, from);
-#if defined(CONFIG_IP_VS) || defined(CONFIG_IP_VS_MODULE)
+#if IS_ENABLED(CONFIG_IP_VS)
to->ipvs_property = from->ipvs_property;
 #endif
skb_copy_secmark(to, from);
-- 
2.7.4

[PATCH 0/8] net: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

Hello David,

This trivial series replace the open coding to check for a Kconfig symbol
being built-in or module, with IS_ENABLED() macro that does exactly that.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Best regards,
Javier


Javier Martinez Canillas (8):
  appletalk: use IS_ENABLED() instead of checking for built-in or module
  lec: use IS_ENABLED() instead of checking for built-in or module
  net: use IS_ENABLED() instead of checking for built-in or module
  ipv4: use IS_ENABLED() instead of checking for built-in or module
  l2tp: use IS_ENABLED() instead of checking for built-in or module
  net: sched: use IS_ENABLED() instead of checking for built-in or
module
  sctp: use IS_ENABLED() instead of checking for built-in or module
  xfrm: use IS_ENABLED() instead of checking for built-in or module

 net/appletalk/ddp.c  |  2 +-
 net/atm/lec.c| 12 ++--
 net/core/dev.c   |  3 +--
 net/ipv4/ip_output.c |  2 +-
 net/l2tp/l2tp_core.h |  2 +-
 net/l2tp/l2tp_eth.c  |  4 ++--
 net/l2tp/l2tp_ppp.c  |  4 ++--
 net/sched/cls_flow.c |  6 +++---
 net/sctp/auth.c  |  2 +-
 net/xfrm/xfrm_algo.c |  2 +-
 10 files changed, 19 insertions(+), 20 deletions(-)

-- 
2.7.4

[PATCH 8/8] xfrm: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 net/xfrm/xfrm_algo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_algo.c b/net/xfrm/xfrm_algo.c
index 250e567ba3d6..44ac85fe2bc9 100644
--- a/net/xfrm/xfrm_algo.c
+++ b/net/xfrm/xfrm_algo.c
@@ -17,7 +17,7 @@
 #include 
 #include 
 #include 
-#if defined(CONFIG_INET_ESP) || defined(CONFIG_INET_ESP_MODULE) || 
defined(CONFIG_INET6_ESP) || defined(CONFIG_INET6_ESP_MODULE)
+#if IS_ENABLED(CONFIG_INET_ESP) || IS_ENABLED(CONFIG_INET6_ESP)
 #include 
 #endif
 
-- 
2.7.4

[PATCH 5/8] l2tp: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/l2tp/l2tp_core.h | 2 +-
 net/l2tp/l2tp_eth.c  | 4 ++--
 net/l2tp/l2tp_ppp.c  | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 5871537af387..2599af6378e4 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -139,7 +139,7 @@ struct l2tp_session {
void (*session_close)(struct l2tp_session *session);
void (*ref)(struct l2tp_session *session);
void (*deref)(struct l2tp_session *session);
-#if defined(CONFIG_L2TP_DEBUGFS) || defined(CONFIG_L2TP_DEBUGFS_MODULE)
+#if IS_ENABLED(CONFIG_L2TP_DEBUGFS)
void (*show)(struct seq_file *m, void *priv);
 #endif
uint8_t priv[0];/* private data */
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 57fc5a46ce06..ef2cd30ca06e 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -195,7 +195,7 @@ static void l2tp_eth_delete(struct l2tp_session *session)
}
 }
 
-#if defined(CONFIG_L2TP_DEBUGFS) || defined(CONFIG_L2TP_DEBUGFS_MODULE)
+#if IS_ENABLED(CONFIG_L2TP_DEBUGFS)
 static void l2tp_eth_show(struct seq_file *m, void *arg)
 {
struct l2tp_session *session = arg;
@@ -268,7 +268,7 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, 
u32 session_id, u32 p
priv->tunnel_sock = tunnel->sock;
session->recv_skb = l2tp_eth_dev_recv;
session->session_close = l2tp_eth_delete;
-#if defined(CONFIG_L2TP_DEBUGFS) || defined(CONFIG_L2TP_DEBUGFS_MODULE)
+#if IS_ENABLED(CONFIG_L2TP_DEBUGFS)
session->show = l2tp_eth_show;
 #endif
 
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 34eff77982cf..41d47bfda15c 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -552,7 +552,7 @@ out:
return error;
 }
 
-#if defined(CONFIG_L2TP_DEBUGFS) || defined(CONFIG_L2TP_DEBUGFS_MODULE)
+#if IS_ENABLED(CONFIG_L2TP_DEBUGFS)
 static void pppol2tp_show(struct seq_file *m, void *arg)
 {
struct l2tp_session *session = arg;
@@ -723,7 +723,7 @@ static int pppol2tp_connect(struct socket *sock, struct 
sockaddr *uservaddr,
 
session->recv_skb   = pppol2tp_recv;
session->session_close  = pppol2tp_session_close;
-#if defined(CONFIG_L2TP_DEBUGFS) || defined(CONFIG_L2TP_DEBUGFS_MODULE)
+#if IS_ENABLED(CONFIG_L2TP_DEBUGFS)
session->show   = pppol2tp_show;
 #endif
 
-- 
2.7.4

[PATCH 7/8] sctp: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/sctp/auth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index 912eb1685a5d..f99d4855d3de 100644
--- a/net/sctp/auth.c
+++ b/net/sctp/auth.c
@@ -48,7 +48,7 @@ static struct sctp_hmac sctp_hmac_list[SCTP_AUTH_NUM_HMACS] = 
{
/* id 2 is reserved as well */
.hmac_id = SCTP_AUTH_HMAC_ID_RESERVED_2,
},
-#if defined (CONFIG_CRYPTO_SHA256) || defined (CONFIG_CRYPTO_SHA256_MODULE)
+#if IS_ENABLED(CONFIG_CRYPTO_SHA256)
{
.hmac_id = SCTP_AUTH_HMAC_ID_SHA256,
.hmac_name = "hmac(sha256)",
-- 
2.7.4

[PATCH 6/8] net: sched: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/sched/cls_flow.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 2c1ae549edbf..a379bae1d74e 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -29,7 +29,7 @@
 #include 
 #include 
 
-#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
 #include 
 #endif
 
@@ -125,14 +125,14 @@ static u32 flow_get_mark(const struct sk_buff *skb)
 
 static u32 flow_get_nfct(const struct sk_buff *skb)
 {
-#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
return addr_fold(skb->nfct);
 #else
return 0;
 #endif
 }
 
-#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
 #define CTTUPLE(skb, member)   \
 ({ \
enum ip_conntrack_info ctinfo;  \
-- 
2.7.4

[PATCH 3/8] net: use IS_ENABLED() instead of checking for built-in or module

2016-09-09 Thread Javier Martinez Canillas

The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 net/core/dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index c7d7db49826d..9e76d4df6ca0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3904,8 +3904,7 @@ static __latent_entropy void net_tx_action(struct 
softirq_action *h)
}
 }
 
-#if (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)) && \
-(defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE))
+#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_ATM_LANE)
 /* This hook is defined here for ATM LANE */
 int (*br_fdb_test_addr_hook)(struct net_device *dev,
 unsigned char *addr) __read_mostly;
-- 
2.7.4

Re: mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

2016-09-06 Thread Javier Martinez Canillas

Hello Kalle,

On 09/03/2016 12:35 PM, Kalle Valo wrote:
> Javier Martinez Canillas <jav...@osg.samsung.com> wrote:
>> If request_irq() fails in mwifiex_sdio_probe_of(), only an error message
>> is printed but the actual error is not propagated to the caller function.
>>
>> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> What's the conclusion with this patch? Should I drop it or take it?
> 
> (The discussion is available from the patchwork link in the signature.)
> 

My understanding is that Arend agrees with the patch and that the question
raised was caused by looking at an older kernel version. IOW, the patch is
OK and should be picked.

I'm adding Arend to cc, so can comment in case I misunderstood him though.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

2016-08-18 Thread Javier Martinez Canillas

Hello Arend,

On 08/18/2016 03:49 PM, Arend van Spriel wrote:
> 
> 
> On 18-08-16 21:29, Javier Martinez Canillas wrote:
>> Hello Arend,
>>
>> Thanks a lot for your feedback.
>>
>> On 08/18/2016 03:14 PM, Arend van Spriel wrote:
>>> On 18-08-16 16:17, Javier Martinez Canillas wrote:
>>>> If request_irq() fails in mwifiex_sdio_probe_of(), only an error message
>>>> is printed but the actual error is not propagated to the caller function.
>>>
>>> Hmm. The caller function, ie. mwifiex_sdio_probe(), does not seem to care.
>>>
>>
>> Hmm, I'm not so sure about that. It's checking the wifiex_sdio_probe_of()
>> return value.
> 
> Ok. I looked at 4.7 sources on lxr [1].
>

Oh, right. That was fixed quite recently indeed.
 
>> If the IRQ request failing is not an error, then at the very least the call
>> to disable_irq() should be avoided if request_irq() fails, and the message
>> should be changed from dev_err() to dev_dgb() or dev_info().
> 
> agree.
> 
>>> The device may still function without this wake interrupt.
>>>
>>
>> That's correct, the binding says that the "interrupts" property in the child
>> node is optional since is just a wakeup IRQ. Now the question is if should
>> be an error if the IRQ is defined but fails to be requested.
> 
> Clearly it indicates an error in the DT specification so behavior is not
> as expected. Personally I would indeed consider it an error, but I was
> just indicating that it might have done like this intentionally.
>

Yes, might had been done intentionally indeed but I don't think that is
the case since the driver lacked error checking and propagation in many
places. But if someone thinks that's better to not honor the DT and at
least have the driver working without the wake up capability, then I'm
happy to respin the patch and change the print log level to info/debug.
 
> Regards,
> Arend
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

2016-08-18 Thread Javier Martinez Canillas

Hello Arend,

Thanks a lot for your feedback.

On 08/18/2016 03:14 PM, Arend van Spriel wrote:
> On 18-08-16 16:17, Javier Martinez Canillas wrote:
>> If request_irq() fails in mwifiex_sdio_probe_of(), only an error message
>> is printed but the actual error is not propagated to the caller function.
> 
> Hmm. The caller function, ie. mwifiex_sdio_probe(), does not seem to care.
>

Hmm, I'm not so sure about that. It's checking the wifiex_sdio_probe_of()
return value.

If the IRQ request failing is not an error, then at the very least the call
to disable_irq() should be avoided if request_irq() fails, and the message
should be changed from dev_err() to dev_dgb() or dev_info().

> The device may still function without this wake interrupt.
>

That's correct, the binding says that the "interrupts" property in the child
node is optional since is just a wakeup IRQ. Now the question is if should
be an error if the IRQ is defined but fails to be requested.

> Regards,
> Arend
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH] mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

2016-08-18 Thread Javier Martinez Canillas

If request_irq() fails in mwifiex_sdio_probe_of(), only an error message
is printed but the actual error is not propagated to the caller function.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index d3e1561ca075..00727936ad6e 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -125,6 +125,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
dev_err(dev,
"Failed to request irq_wifi %d (%d)\n",
cfg->irq_wifi, ret);
+   return ret;
}
disable_irq(cfg->irq_wifi);
}
-- 
2.5.5

Re: [PATCH] mwifiex: fix unconditional error return in .add_virtual_intf callback

2016-07-05 Thread Javier Martinez Canillas

Hello Kalle,

On 07/05/2016 09:09 AM, Kalle Valo wrote:
> Javier Martinez Canillas <jav...@osg.samsung.com> writes:
> 
>> The commit 7311ea850079 ("mwifiex: fix AP start problem for newly added
>> interface") attempted to fix an issue when a new AP interface is added.
>>
>> But the patch didn't check the return value of the functions doing the
>> firmware calls and returned an error even if the functions didn't fail.
>>
>> This prevents the network device to be registered properly, so fix it.
>>
>> Fixes: commit 7311ea850079 ("mwifiex: fix AP start problem for newly added 
>> interface")
>> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> The fixes line should be:
> 
> Fixes: 7311ea850079 ("mwifiex: fix AP start problem for newly added 
> interface")
> 
> I can fix that before I apply the patch.
> 

Sigh, it was a copy and paste error when I copied the SHA-1 from the
commit message. Sorry about that and thanks for taking care of this.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH] mwifiex: fix unconditional error return in .add_virtual_intf callback

2016-07-01 Thread Javier Martinez Canillas

The commit 7311ea850079 ("mwifiex: fix AP start problem for newly added
interface") attempted to fix an issue when a new AP interface is added.

But the patch didn't check the return value of the functions doing the
firmware calls and returned an error even if the functions didn't fail.

This prevents the network device to be registered properly, so fix it.

Fixes: commit 7311ea850079 ("mwifiex: fix AP start problem for newly added 
interface")
Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 99e8cf1ad610..5de9f63e2c01 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -2865,9 +2865,11 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct 
wiphy *wiphy,
 
ret = mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE,
   HostCmd_ACT_GEN_SET, 0, NULL, true);
+   if (ret)
return ERR_PTR(ret);
 
ret = mwifiex_sta_init_cmd(priv, false, false);
+   if (ret)
return ERR_PTR(ret);
 
mwifiex_setup_ht_caps(>bands[NL80211_BAND_2GHZ]->ht_cap, priv);
-- 
2.5.5

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

2016-06-22 Thread Javier Martinez Canillas

Hello Kalle,

On 06/22/2016 02:17 AM, Kalle Valo wrote:
> Javier Martinez Canillas <jav...@osg.samsung.com> writes:
> 
>>>> Patch 3/3 applies cleanly even after dropping patch 2/3.
>>>> Is that Ok for you or do you want me to re-resend a v3
>>>> with only patches 1/3 and 3/3?
>>>
>>> I can drop patch 2, no need to resend. Thanks.
>>>
>>
>> I saw that you sent your pull request for v4.8 but the
>> patches from this series were not included:
>>
>> https://lkml.org/lkml/2016/6/21/400
> 
> I had forgotten to change the state of patches 1 and 3 from 'Changes
> Requested' back to 'New' and that's why I missed them. I did that now so
> they are back in my queue:
> 
> https://patchwork.kernel.org/patch/9158855/
> https://patchwork.kernel.org/patch/9158851/
> 
> Thanks for checking.
> 

Ok, thanks for the information.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

2016-06-21 Thread Javier Martinez Canillas

Hello Kalle,

On 06/10/2016 03:54 PM, Kalle Valo wrote:
> Javier Martinez Canillas <jav...@osg.samsung.com> writes:
> 
>>> This patch (2/3) is only for code rearrangement and adds an
>>> unnecessary wrapper function. We can simply drop the patch.
>>
>> Agreed.
>>
>> Kalle,
>>
>> Patch 3/3 applies cleanly even after dropping patch 2/3.
>> Is that Ok for you or do you want me to re-resend a v3
>> with only patches 1/3 and 3/3?
> 
> I can drop patch 2, no need to resend. Thanks.
> 

I saw that you sent your pull request for v4.8 but the
patches from this series were not included:

https://lkml.org/lkml/2016/6/21/400

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

2016-06-10 Thread Javier Martinez Canillas

Hello Amitkumar,

On 06/10/2016 12:26 PM, Amitkumar Karwar wrote:
> Hi Kalle/Javier,
> 
>> From: Javier Martinez Canillas [mailto:jav...@osg.samsung.com]
>> Sent: Friday, June 10, 2016 8:07 PM
>> To: Kalle Valo
>> Cc: linux-ker...@vger.kernel.org; Julian Calaby; Shengzhen Li; Enric
>> Balletbo i Serra; Amitkumar Karwar; netdev@vger.kernel.org; linux-
>> wirel...@vger.kernel.org; Nishant Sarmukadam
>> Subject: Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station
>> ioctl file
>>
>> Hello Kalle,
>>
>> On 06/10/2016 10:30 AM, Kalle Valo wrote:
>>> Javier Martinez Canillas <jav...@osg.samsung.com> writes:
>>>
>>>> From: Shengzhen Li <s...@marvell.com>
>>>>
>>>> Most cfg80211 operations are just a wrappers to functions defined in
>>>> the sta_ioctl.c file, so for consistency move the .get_tx_power logic
>> there.
>>>>
>>>> Signed-off-by: Shengzhen Li <s...@marvell.com>
>>>> Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
>>>> [javier: update the subject line and commit message]
>>>> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
>>>
>>> [...]
>>>
>>>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>>> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy
>> *wiphy,
>>>>  int *dbm)
>>>>  {
>>>>struct mwifiex_adapter *adapter =
>> mwifiex_cfg80211_get_adapter(wiphy);
>>>> -  struct mwifiex_private *priv = mwifiex_get_priv(adapter,
>>>> -  MWIFIEX_BSS_ROLE_ANY);
>>>> -  int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
>>>> - HostCmd_ACT_GEN_GET, 0, NULL, true);
>>>> -
>>>> -  if (ret < 0)
>>>> -  return ret;
>>>> -
>>>> -  /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler
>> */
>>>> -  *dbm = priv->tx_power_level;
>>>> +  struct mwifiex_private *priv;
>>>>
>>>> -  return 0;
>>>> +  priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>>>> +  return mwifiex_get_tx_power(priv, dbm);
>>>>  }
>>>
>>> So in patch 1 you added the patch and in patch 2 you move it to a
>>> different location? That doesn't make any sense, can't you just fold
>>> the two patches into one so that the function is added only once.
>>>
>>
>> I posted this patch in v1 but then Amitkumar shared his patch with me
>> that was very similar to mine, only that the logic was in a different
>> location.
>>
>> So I included his delta as a separate patch to try keeping attribution
>> as best as possible.
>>
> 
> This patch (2/3) is only for code rearrangement and adds an unnecessary 
> wrapper function. We can simply drop the patch.
>

Agreed.

Kalle,

Patch 3/3 applies cleanly even after dropping patch 2/3.
Is that Ok for you or do you want me to re-resend a v3
with only patches 1/3 and 3/3?

> Regards,
> Amitkumar
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

2016-06-10 Thread Javier Martinez Canillas

Hello Kalle,

On 06/10/2016 10:30 AM, Kalle Valo wrote:
> Javier Martinez Canillas <jav...@osg.samsung.com> writes:
> 
>> From: Shengzhen Li <s...@marvell.com>
>>
>> Most cfg80211 operations are just a wrappers to functions defined in the
>> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>>
>> Signed-off-by: Shengzhen Li <s...@marvell.com>
>> Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
>> [javier: update the subject line and commit message]
>> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> [...]
> 
>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
>>int *dbm)
>>  {
>>  struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
>> -struct mwifiex_private *priv = mwifiex_get_priv(adapter,
>> -MWIFIEX_BSS_ROLE_ANY);
>> -int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
>> -   HostCmd_ACT_GEN_GET, 0, NULL, true);
>> -
>> -if (ret < 0)
>> -return ret;
>> -
>> -/* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
>> -*dbm = priv->tx_power_level;
>> +struct mwifiex_private *priv;
>>  
>> -return 0;
>> +priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>> +return mwifiex_get_tx_power(priv, dbm);
>>  }
> 
> So in patch 1 you added the patch and in patch 2 you move it to a
> different location? That doesn't make any sense, can't you just fold the
> two patches into one so that the function is added only once.
>

I posted this patch in v1 but then Amitkumar shared his patch with me that
was very similar to mine, only that the logic was in a different location.

So I included his delta as a separate patch to try keeping attribution as
best as possible.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH v2 3/3] mwifiex: add get_antenna support for cfg80211

2016-06-06 Thread Javier Martinez Canillas

From: Shengzhen Li <s...@marvell.com>

Since commit de3bb771f471 ("cfg80211: add more warnings for inconsistent
ops") the wireless core warns if a driver implements a cfg80211 callback
but doesn't implements the inverse operation.

The mwifiex driver defines a .set_antenna handler but not a .get_antenna
so this not only makes the core to print a warning when creating a new
wiphy but also the antenna isn't reported to user-space apps such as iw.

This patch queries the antenna to the firmware so is properly reported to
user-space. With this patch, the wireless core does not warn anymore and:

$ iw phy phy0 info | grep Antennas
Available Antennas: TX 0x3 RX 0x3
Configured Antennas: TX 0x3 RX 0x3

Signed-off-by: Shengzhen Li <s...@marvell.com>
Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
[javier: expand the commit message]
Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c| 16 +++
 drivers/net/wireless/marvell/mwifiex/fw.h  |  3 ++
 drivers/net/wireless/marvell/mwifiex/init.c|  2 +
 drivers/net/wireless/marvell/mwifiex/main.h|  2 +
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 50 +++---
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 10 +++--
 6 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ff3f63ed95e1..ff1eefe5087b 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1819,6 +1819,21 @@ mwifiex_cfg80211_set_antenna(struct wiphy *wiphy, u32 
tx_ant, u32 rx_ant)
HostCmd_ACT_GEN_SET, 0, _cfg, true);
 }
 
+static int
+mwifiex_cfg80211_get_antenna(struct wiphy *wiphy, u32 *tx_ant, u32 *rx_ant)
+{
+   struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
+   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
+   MWIFIEX_BSS_ROLE_ANY);
+   mwifiex_send_cmd(priv, HostCmd_CMD_RF_ANTENNA,
+HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+   *tx_ant = priv->tx_ant;
+   *rx_ant = priv->rx_ant;
+
+   return 0;
+}
+
 /* cfg80211 operation handler for stop ap.
  * Function stops BSS running at uAP interface.
  */
@@ -3962,6 +3977,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
.change_beacon = mwifiex_cfg80211_change_beacon,
.set_cqm_rssi_config = mwifiex_cfg80211_set_cqm_rssi_config,
.set_antenna = mwifiex_cfg80211_set_antenna,
+   .get_antenna = mwifiex_cfg80211_get_antenna,
.del_station = mwifiex_cfg80211_del_station,
.sched_scan_start = mwifiex_cfg80211_sched_scan_start,
.sched_scan_stop = mwifiex_cfg80211_sched_scan_stop,
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h 
b/drivers/net/wireless/marvell/mwifiex/fw.h
index 8e4145abdbfa..cef72343f5b6 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -462,6 +462,9 @@ enum P2P_MODES {
 #define HostCmd_ACT_SET_RX  0x0001
 #define HostCmd_ACT_SET_TX  0x0002
 #define HostCmd_ACT_SET_BOTH0x0003
+#define HostCmd_ACT_GET_RX  0x0004
+#define HostCmd_ACT_GET_TX  0x0008
+#define HostCmd_ACT_GET_BOTH0x000c
 
 #define RF_ANTENNA_AUTO 0x
 
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c 
b/drivers/net/wireless/marvell/mwifiex/init.c
index 78c532f0d286..fbaf49056746 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -110,6 +110,8 @@ int mwifiex_init_priv(struct mwifiex_private *priv)
priv->tx_power_level = 0;
priv->max_tx_power_level = 0;
priv->min_tx_power_level = 0;
+   priv->tx_ant = 0;
+   priv->rx_ant = 0;
priv->tx_rate = 0;
priv->rxpd_htinfo = 0;
priv->rxpd_rate = 0;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
b/drivers/net/wireless/marvell/mwifiex/main.h
index 79c28cfb7780..2ae7ff74e1c6 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -533,6 +533,8 @@ struct mwifiex_private {
u16 tx_power_level;
u8 max_tx_power_level;
u8 min_tx_power_level;
+   u32 tx_ant;
+   u32 rx_ant;
u8 tx_rate;
u8 tx_htinfo;
u8 rxpd_htinfo;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c 
b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index e436574b1698..8c658495bf66 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -313,23 +313,41 @@ static int mwifiex_cmd_rf_antenna(struct mwifiex_private 
*priv,
 
cmd->comm

[PATCH v2 0/3] mwifiex: add .get_tx_power and .get_antenna cfg80211 operations

2016-06-06 Thread Javier Martinez Canillas

Hello,

Since commit de3bb771f471 ("cfg80211: add more warnings for inconsistent
ops") the wireless core warns if a driver implements a cfg80211 callback
but doesn't implements the inverse operation. The mwifiex driver has two
set operations defined without the get counterpat so warnings are shown:

WARNING: CPU: 3 PID: 127 at net/wireless/core.c:366 wiphy_new_nm+0x66c/0x6ac
Modules linked in: mwifiex_sdio mwifiex
CPU: 3 PID: 127 Comm: kworker/3:1 Tainted: GW   
4.7.0-rc1-next-20160531-6-g569df5b983f3
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events request_firmware_work_func
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x88/0x9c)
[] (dump_stack) from [] (__warn+0xe8/0x100)
[] (__warn) from [] (warn_slowpath_null+0x20/0x28)
[] (warn_slowpath_null) from [] 
(wiphy_new_nm+0x66c/0x6ac)
[] (wiphy_new_nm) from [] 
(mwifiex_register_cfg80211+0x28/0x3f0 [mwifiex])
[] (mwifiex_register_cfg80211 [mwifiex]) from [] 
(mwifiex_fw_dpc+0x2b0/0x474 [mwifiex])
[] (mwifiex_fw_dpc [mwifiex]) from [] 
(request_firmware_work_func+0x30/0x58)
[] (request_firmware_work_func) from [] 
(process_one_work+0x124/0x338)
[] (process_one_work) from [] (worker_thread+0x38/0x4d4)
[] (worker_thread) from [] (kthread+0xdc/0xf4)
[] (kthread) from [] (ret_from_fork+0x14/0x3c)

This patch series fixes the warnings by implementing callbacks for the
missing get operations.

Since the first version was posted [0], Amitkumar Karwar from Marvell
provided me their patches that adds the same. The .get_tx_power patch was
very similar to mine. So I kept my patch and added the delta as a follow
up patch but discarded my .get_antenna patch since the one provided by
Amitkumar queries the firmware so is the correct approach.

Patches were tested on an Exynos5800 Chromebook that has a mwifiex chip.

Changes since v1:
- Drop patch that reports antenna cached values and use Marvell's patch
  instead.
- Add patch that moves the .get_tx_power() logic to sta_ioctl.c file.

[0]: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1160119.html

Best regards,
Javier


Javier Martinez Canillas (1):
  mwifiex: add a cfg80211 .get_tx_power operation callback

Shengzhen Li (2):
  mwifiex: move .get_tx_power logic to station ioctl file
  mwifiex: add get_antenna support for cfg80211

 drivers/net/wireless/marvell/mwifiex/cfg80211.c| 32 ++
 drivers/net/wireless/marvell/mwifiex/fw.h  |  3 ++
 drivers/net/wireless/marvell/mwifiex/init.c|  2 +
 drivers/net/wireless/marvell/mwifiex/main.h|  4 ++
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 50 +++---
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 10 +++--
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c   | 18 
 7 files changed, 100 insertions(+), 19 deletions(-)

-- 
2.5.5

[PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

2016-06-06 Thread Javier Martinez Canillas

From: Shengzhen Li <s...@marvell.com>

Most cfg80211 operations are just a wrappers to functions defined in the
sta_ioctl.c file, so for consistency move the .get_tx_power logic there.

Signed-off-by: Shengzhen Li <s...@marvell.com>
Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
[javier: update the subject line and commit message]
Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c  | 14 +++---
 drivers/net/wireless/marvell/mwifiex/main.h  |  2 ++
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 18 ++
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b17f3d09a2c7..ff3f63ed95e1 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
  int *dbm)
 {
struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
-   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
-   MWIFIEX_BSS_ROLE_ANY);
-   int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
-  HostCmd_ACT_GEN_GET, 0, NULL, true);
-
-   if (ret < 0)
-   return ret;
-
-   /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
-   *dbm = priv->tx_power_level;
+   struct mwifiex_private *priv;
 
-   return 0;
+   priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
+   return mwifiex_get_tx_power(priv, dbm);
 }
 
 /*
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
b/drivers/net/wireless/marvell/mwifiex/main.h
index 0207af00be42..79c28cfb7780 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1464,6 +1464,8 @@ int mwifiex_drv_get_driver_version(struct mwifiex_adapter 
*adapter,
 int mwifiex_set_tx_power(struct mwifiex_private *priv,
 struct mwifiex_power_cfg *power_cfg);
 
+int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm);
+
 int mwifiex_main_process(struct mwifiex_adapter *);
 
 int mwifiex_queue_tx_pkt(struct mwifiex_private *priv, struct sk_buff *skb);
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c 
b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index 8e0862657122..70ff9b805b5b 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -775,6 +775,24 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
 }
 
 /*
+ * IOCTL request handler to get tx power configuration.
+ *
+ * This function prepares the correct firmware command and
+ * issues it.
+ */
+int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm)
+{
+   int ret;
+
+   ret = mwifiex_send_cmd(priv, HostCmd_CMD_TXPWR_CFG,
+  HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+   *dbm = priv->tx_power_level;
+
+   return ret;
+}
+
+/*
  * IOCTL request handler to get power save mode.
  *
  * This function prepares the correct firmware command and
-- 
2.5.5

[PATCH v2 1/3] mwifiex: add a cfg80211 .get_tx_power operation callback

2016-06-06 Thread Javier Martinez Canillas

The mwifiex driver implements a cfg80211 .set_tx_power operation handler
but doesn't have the inverse .get_tx_power callback.

This not only has the effect that the Tx power can't be reported to user
space tools such as iwconfig and iwlist but also that the wireless core
prints a warning when a new wiphy is created due an cfg80211 operation
being implemented without its counterpart.

After this patch, the Tx power is properly reported to user-space tools:

$ iwlist mlan0 txpower
mlan0 unknown transmit-power information.

  Current Tx-Power=13 dBm   (19 mW)

and also the following warning isn't shown anymore on the driver probe:

WARNING: CPU: 3 PID: 127 at net/wireless/core.c:366 wiphy_new_nm+0x66c/0x6ac
Modules linked in: mwifiex_sdio mwifiex
CPU: 3 PID: 127 Comm: kworker/3:1 Tainted: GW   
4.7.0-rc1-next-20160531-6-g569df5b983f3
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events request_firmware_work_func
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x88/0x9c)
[] (dump_stack) from [] (__warn+0xe8/0x100)
[] (__warn) from [] (warn_slowpath_null+0x20/0x28)
[] (warn_slowpath_null) from [] (wiphy_new_nm+0x66c/0x6ac)
[] (wiphy_new_nm) from [] 
(mwifiex_register_cfg80211+0x28/0x3f0 [mwifiex])
[] (mwifiex_register_cfg80211 [mwifiex]) from [] 
(mwifiex_fw_dpc+0x2b0/0x474 [mwifiex])
[] (mwifiex_fw_dpc [mwifiex]) from [] 
(request_firmware_work_func+0x30/0x58)
[] (request_firmware_work_func) from [] 
(process_one_work+0x124/0x338)
[] (process_one_work) from [] (worker_thread+0x38/0x4d4)
[] (worker_thread) from [] (kthread+0xdc/0xf4)
[] (kthread) from [] (ret_from_fork+0x14/0x3c)

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ff948a92..b17f3d09a2c7 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -377,6 +377,29 @@ mwifiex_cfg80211_set_tx_power(struct wiphy *wiphy,
 }
 
 /*
+ * CFG802.11 operation handler to get Tx power.
+ */
+static int
+mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ int *dbm)
+{
+   struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
+   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
+   MWIFIEX_BSS_ROLE_ANY);
+   int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
+  HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+   if (ret < 0)
+   return ret;
+
+   /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
+   *dbm = priv->tx_power_level;
+
+   return 0;
+}
+
+/*
  * CFG802.11 operation handler to set Power Save option.
  *
  * The timeout value, if provided, is currently ignored.
@@ -3940,6 +3963,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
.set_default_key = mwifiex_cfg80211_set_default_key,
.set_power_mgmt = mwifiex_cfg80211_set_power_mgmt,
.set_tx_power = mwifiex_cfg80211_set_tx_power,
+   .get_tx_power = mwifiex_cfg80211_get_tx_power,
.set_bitrate_mask = mwifiex_cfg80211_set_bitrate_mask,
.start_ap = mwifiex_cfg80211_start_ap,
.stop_ap = mwifiex_cfg80211_stop_ap,
-- 
2.5.5

[PATCH 1/2] mwifiex: add a cfg80211 .get_tx_power operation callback

2016-06-06 Thread Javier Martinez Canillas

The mwifiex driver implements a cfg80211 .set_tx_power operation handler
but doesn't have the inverse .get_tx_power callback.

This not only has the effect that the Tx power can't be reported to user
space tools such as iwconfig and iwlist but also that the wireless core
prints a warning when a new wiphy is created due an cfg80211 operation
being implemented without its counterpart.

After this patch, the Tx power is properly reported to user-space tools:

$ iwlist mlan0 txpower
mlan0 unknown transmit-power information.

  Current Tx-Power=13 dBm   (19 mW)

and also the following warning isn't shown anymore on the driver probe:

WARNING: CPU: 3 PID: 127 at net/wireless/core.c:366 wiphy_new_nm+0x66c/0x6ac
Modules linked in: mwifiex_sdio mwifiex
CPU: 3 PID: 127 Comm: kworker/3:1 Tainted: GW   
4.7.0-rc1-next-20160531-6-g569df5b983f3
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events request_firmware_work_func
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x88/0x9c)
[] (dump_stack) from [] (__warn+0xe8/0x100)
[] (__warn) from [] (warn_slowpath_null+0x20/0x28)
[] (warn_slowpath_null) from [] (wiphy_new_nm+0x66c/0x6ac)
[] (wiphy_new_nm) from [] 
(mwifiex_register_cfg80211+0x28/0x3f0 [mwifiex])
[] (mwifiex_register_cfg80211 [mwifiex]) from [] 
(mwifiex_fw_dpc+0x2b0/0x474 [mwifiex])
[] (mwifiex_fw_dpc [mwifiex]) from [] 
(request_firmware_work_func+0x30/0x58)
[] (request_firmware_work_func) from [] 
(process_one_work+0x124/0x338)
[] (process_one_work) from [] (worker_thread+0x38/0x4d4)
[] (worker_thread) from [] (kthread+0xdc/0xf4)
[] (kthread) from [] (ret_from_fork+0x14/0x3c)

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ff948a92..b17f3d09a2c7 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -377,6 +377,29 @@ mwifiex_cfg80211_set_tx_power(struct wiphy *wiphy,
 }
 
 /*
+ * CFG802.11 operation handler to get Tx power.
+ */
+static int
+mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ int *dbm)
+{
+   struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
+   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
+   MWIFIEX_BSS_ROLE_ANY);
+   int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
+  HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+   if (ret < 0)
+   return ret;
+
+   /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
+   *dbm = priv->tx_power_level;
+
+   return 0;
+}
+
+/*
  * CFG802.11 operation handler to set Power Save option.
  *
  * The timeout value, if provided, is currently ignored.
@@ -3940,6 +3963,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
.set_default_key = mwifiex_cfg80211_set_default_key,
.set_power_mgmt = mwifiex_cfg80211_set_power_mgmt,
.set_tx_power = mwifiex_cfg80211_set_tx_power,
+   .get_tx_power = mwifiex_cfg80211_get_tx_power,
.set_bitrate_mask = mwifiex_cfg80211_set_bitrate_mask,
.start_ap = mwifiex_cfg80211_start_ap,
.stop_ap = mwifiex_cfg80211_stop_ap,
-- 
2.5.5

[PATCH 0/2] mwifiex: add .get_tx_power and .get_antenna cfg80211 operations

2016-06-06 Thread Javier Martinez Canillas

Hello,

Since commit de3bb771f471 ("cfg80211: add more warnings for inconsistent
ops") the wireless core warns if a driver implements a cfg80211 callback
but doesn't implements the inverse operation. The mwifiex driver has two
set operations defined without the get counterpat so warnings are shown:

WARNING: CPU: 3 PID: 127 at net/wireless/core.c:366 wiphy_new_nm+0x66c/0x6ac
Modules linked in: mwifiex_sdio mwifiex
CPU: 3 PID: 127 Comm: kworker/3:1 Tainted: GW   
4.7.0-rc1-next-20160531-6-g569df5b983f3
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events request_firmware_work_func
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x88/0x9c)
[] (dump_stack) from [] (__warn+0xe8/0x100)
[] (__warn) from [] (warn_slowpath_null+0x20/0x28)
[] (warn_slowpath_null) from [] (wiphy_new_nm+0x66c/0x6ac)
[] (wiphy_new_nm) from [] 
(mwifiex_register_cfg80211+0x28/0x3f0 [mwifiex])
[] (mwifiex_register_cfg80211 [mwifiex]) from [] 
(mwifiex_fw_dpc+0x2b0/0x474 [mwifiex])
[] (mwifiex_fw_dpc [mwifiex]) from [] 
(request_firmware_work_func+0x30/0x58)
[] (request_firmware_work_func) from [] 
(process_one_work+0x124/0x338)
[] (process_one_work) from [] (worker_thread+0x38/0x4d4)
[] (worker_thread) from [] (kthread+0xdc/0xf4)
[] (kthread) from [] (ret_from_fork+0x14/0x3c)

This patch series fixes the warnings by implementing callbacks for the
get operations.

There isn't public documentation about the firmware interface so the
data is only queried to the firmware if the driver already supports
it as is the case of the Tx power. For the antenna the driver only
supports the get command so the get returns the cached values that
were previously set. This can be changed to query the firmware as a
followup by someone with access to the docs, if this is supported.

Best regards,
Javier


Javier Martinez Canillas (2):
  mwifiex: add a cfg80211 .get_tx_power operation callback
  mwifiex: add a cfg80211 .get_antenna operation callback

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 50 -
 drivers/net/wireless/marvell/mwifiex/main.h |  1 +
 2 files changed, 49 insertions(+), 2 deletions(-)

-- 
2.5.5

[PATCH 2/2] mwifiex: add a cfg80211 .get_antenna operation callback

2016-06-06 Thread Javier Martinez Canillas

Since commit de3bb771f471 ("cfg80211: add more warnings for inconsistent
ops") the wireless core warns if a driver implements a cfg80211 callback
but doesn't implements the inverse operation.

The mwifiex driver defines a .set_antenna handler but not a .get_antenna
so this not only makes the core to print a warning when creating a new
wiphy but also the antenna isn't reported to user-space apps such as iw.

Unfortunately the driver doesn't have support to query the antena to the
firmware and there isn't public documentation about the interface so this
patch caches the values set in .set_antenna and reports those back in get.

With this patch, the wireless core does not warn anymore and the antenna
is properly reported once has been set:

$ iw phy phy0 set antenna 0x1

$ iw phy phy0 info | grep Antennas
Available Antennas: TX 0x3 RX 0x3
Configured Antennas: TX 0x1 RX 0x1

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---
Hello,

Even though this approach prevents the warning and allows to reports the
antenna values, it would be better if instead of caching the set values,
these are asked to the firmware for each .get_antenna callback.

But the driver currently only implements a set antenna command and the
firmware interface documentation is not public so isn't clear if the
firmware doesn't support or is just that the driver didn't implement it.

Best regards,
Javier

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 26 +++--
 drivers/net/wireless/marvell/mwifiex/main.h |  1 +
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b17f3d09a2c7..e9efbc852d23 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1771,6 +1771,7 @@ mwifiex_cfg80211_set_antenna(struct wiphy *wiphy, u32 
tx_ant, u32 rx_ant)
struct mwifiex_private *priv = mwifiex_get_priv(adapter,
MWIFIEX_BSS_ROLE_ANY);
struct mwifiex_ds_ant_cfg ant_cfg;
+   int ret;
 
if (!tx_ant || !rx_ant)
return -EOPNOTSUPP;
@@ -1823,8 +1824,28 @@ mwifiex_cfg80211_set_antenna(struct wiphy *wiphy, u32 
tx_ant, u32 rx_ant)
ant_cfg.tx_ant = tx_ant;
ant_cfg.rx_ant = rx_ant;
 
-   return mwifiex_send_cmd(priv, HostCmd_CMD_RF_ANTENNA,
-   HostCmd_ACT_GEN_SET, 0, _cfg, true);
+   ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_ANTENNA,
+  HostCmd_ACT_GEN_SET, 0, _cfg, true);
+
+   if (ret < 0)
+   return ret;
+
+   priv->ant_cfg.tx_ant = tx_ant;
+   priv->ant_cfg.rx_ant = rx_ant;
+
+   return 0;
+}
+
+static int
+mwifiex_cfg80211_get_antenna(struct wiphy *wiphy, u32 *tx_ant, u32 *rx_ant)
+{
+   struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
+   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
+   MWIFIEX_BSS_ROLE_ANY);
+   *tx_ant = priv->ant_cfg.tx_ant;
+   *rx_ant = priv->ant_cfg.rx_ant;
+
+   return 0;
 }
 
 /* cfg80211 operation handler for stop ap.
@@ -3970,6 +3991,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
.change_beacon = mwifiex_cfg80211_change_beacon,
.set_cqm_rssi_config = mwifiex_cfg80211_set_cqm_rssi_config,
.set_antenna = mwifiex_cfg80211_set_antenna,
+   .get_antenna = mwifiex_cfg80211_get_antenna,
.del_station = mwifiex_cfg80211_del_station,
.sched_scan_start = mwifiex_cfg80211_sched_scan_start,
.sched_scan_stop = mwifiex_cfg80211_sched_scan_stop,
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
b/drivers/net/wireless/marvell/mwifiex/main.h
index 0207af00be42..b4fe10cbb34d 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -668,6 +668,7 @@ struct mwifiex_private {
struct delayed_work dfs_chan_sw_work;
struct cfg80211_beacon_data beacon_after;
struct mwifiex_11h_intf_state state_11h;
+   struct mwifiex_ds_ant_cfg ant_cfg;
struct mwifiex_ds_mem_rw mem_rw;
struct sk_buff_head bypass_txq;
struct mwifiex_user_scan_chan hidden_chan[MWIFIEX_USER_SCAN_CHAN_MAX];
-- 
2.5.5

Re: [PATCH 7/8] mwifiex: don't print an error if an optional DT property is missing

2016-06-01 Thread Javier Martinez Canillas

Hello Julian,

Thanks a lot for your feedback and reviews.

On 06/01/2016 12:20 AM, Julian Calaby wrote:
> Hi All,
> 
> On Sat, May 28, 2016 at 12:18 AM, Javier Martinez Canillas
> <jav...@osg.samsung.com> wrote:
>> The Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt DT
>> binding document say that the "interrupts" property in the child node is
>> optional. So the property being missed shouldn't be treated as an error.
> 
> Have you checked whether it is truly optional? I.e. nothing else
> breaks if this property isn't set?
>

That's what the DT binding says and the IRQ is only used as a wakeup source
during system suspend, it is not used during runtime. And that is why the
mwifiex_sdio_probe_of() function does not fail if the IRQ is missing.

Now, I just got to that conclusion by reading the binding docs, the message
in the commits that introduced this and the driver code. Xinming Hu should
comment on how critical this feature is for systems that needs to be wakeup.

In any case I think that the code should be consistent with what the binding
doc says and also the function does (i.e: dev_err only if returns an error).

>> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> 
> Other than that, this looks sensible to me.
> 
> Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH 2/8] mwifiex: propagate sdio_enable_func() errno code in mwifiex_sdio_probe()

2016-05-27 Thread Javier Martinez Canillas

If the sdio_enable_func() function fails on .probe, the -EIO errno code
is always returned but that could make more difficult to debug and find
the cause of why the function actually failed.

Since the driver/device core prints the value returned by .probe in its
error message propagate what was returned by sdio_enable_func() at fail.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 285b1b68f7e9..ab64507c84e1 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -184,7 +184,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
if (ret) {
pr_err("%s: failed to enable function\n", __func__);
kfree(card);
-   return -EIO;
+   return ret;
}
 
/* device tree node parsing and platform specific configuration*/
-- 
2.5.5

[PATCH 3/8] mwifiex: propagate mwifiex_add_card() errno code in mwifiex_sdio_probe()

2016-05-27 Thread Javier Martinez Canillas

There's only a check if mwifiex_add_card() returned a nonzero value, but
the actual error code is neither stored nor propagated to the caller. So
instead of always returning -1 (which is -EPERM and not a suitable errno
code in this case), propagate the value returned by mwifiex_add_card().

Patch also removes the assignment of sdio_disable_func() returned value
since it was overwritten anyways and what matters is to know the error
value returned by the first function that failed.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index ab64507c84e1..81003fbe5025 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -191,14 +191,14 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
if (func->dev.of_node)
mwifiex_sdio_probe_of(>dev, card);
 
-   if (mwifiex_add_card(card, _remove_card_sem, _ops,
-MWIFIEX_SDIO)) {
+   ret = mwifiex_add_card(card, _remove_card_sem, _ops,
+  MWIFIEX_SDIO);
+   if (ret) {
pr_err("%s: add card failed\n", __func__);
kfree(card);
sdio_claim_host(func);
-   ret = sdio_disable_func(func);
+   sdio_disable_func(func);
sdio_release_host(func);
-   ret = -1;
}
 
return ret;
-- 
2.5.5

[PATCH 1/8] mwifiex: only call mwifiex_sdio_probe_of() if dev has an OF node

2016-05-27 Thread Javier Martinez Canillas

SDIO is an auto enumerable bus so the SDIO devices are matched using the
sdio_device_id table and not using compatible strings from a OF id table.

However, commit ce4f6f0c353b ("mwifiex: add platform specific wakeup
interrupt support") allowed to match nodes defined as child of the SDIO
host controller in the probe function using a compatible string to setup
platform specific parameters in the DT.

The problem is that the OF parse function is always called regardless if
the SDIO dev has an OF node associated or not, and prints an error if it
is not found. So, on a platform that doesn't have a node for a SDIO dev,
the following misleading error message will be printed:

[  12.480042] mwifiex_sdio mmc2:0001:1: sdio platform data not available

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index bdc51ffd43ec..285b1b68f7e9 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -102,8 +102,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
struct mwifiex_plt_wake_cfg *cfg;
int ret;
 
-   if (!dev->of_node ||
-   !of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
+   if (!of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
dev_err(dev, "sdio platform data not available\n");
return -1;
}
@@ -189,7 +188,8 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
}
 
/* device tree node parsing and platform specific configuration*/
-   mwifiex_sdio_probe_of(>dev, card);
+   if (func->dev.of_node)
+   mwifiex_sdio_probe_of(>dev, card);
 
if (mwifiex_add_card(card, _remove_card_sem, _ops,
 MWIFIEX_SDIO)) {
-- 
2.5.5

[PATCH 0/8] mwifiex: Fix some error handling issues in mwifiex_sdio_probe() function

2016-05-27 Thread Javier Martinez Canillas

Hello,

While booting a system with a mwifiex WiFi card, I noticed the following
missleading error message:

[  12.480042] mwifiex_sdio mmc2:0001:1: sdio platform data not available

This error only applies to platforms that define a child node for the SDIO
device, but it's currently shown even in platforms that don't have a child
node defined.

So this series fixes this issue and others I found in the .probe function
(mostly related to error handling and the error path) while looking at it.

Best regards,
Javier


Javier Martinez Canillas (8):
  mwifiex: only call mwifiex_sdio_probe_of() if dev has an OF node
  mwifiex: propagate sdio_enable_func() errno code in
mwifiex_sdio_probe()
  mwifiex: propagate mwifiex_add_card() errno code in
mwifiex_sdio_probe()
  mwifiex: consolidate mwifiex_sdio_probe() error paths
  mwifiex: use dev_err() instead of pr_err() in mwifiex_sdio_probe()
  mwifiex: check if mwifiex_sdio_probe_of() fails and return error
  mwifiex: don't print an error if an optional DT property is missing
  mwifiex: use better message and error code when OF node doesn't match

 drivers/net/wireless/marvell/mwifiex/sdio.c | 46 ++---
 1 file changed, 28 insertions(+), 18 deletions(-)

-- 
2.5.5

[PATCH 4/8] mwifiex: consolidate mwifiex_sdio_probe() error paths

2016-05-27 Thread Javier Martinez Canillas

Instead of duplicating part of the cleanups needed in case of an error
in .probe callback, have a single error path and use goto labels as is
common practice in the kernel.

This also has the nice side effect that the cleanup operations are made
in the inverse order of their counterparts, which was not the case for
the mwifiex_add_card() error path.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 81003fbe5025..7aeee88b858f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -183,8 +183,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
 
if (ret) {
pr_err("%s: failed to enable function\n", __func__);
-   kfree(card);
-   return ret;
+   goto err_free;
}
 
/* device tree node parsing and platform specific configuration*/
@@ -195,12 +194,18 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
   MWIFIEX_SDIO);
if (ret) {
pr_err("%s: add card failed\n", __func__);
-   kfree(card);
-   sdio_claim_host(func);
-   sdio_disable_func(func);
-   sdio_release_host(func);
+   goto err_disable;
}
 
+   return 0;
+
+err_disable:
+   sdio_claim_host(func);
+   sdio_disable_func(func);
+   sdio_release_host(func);
+err_free:
+   kfree(card);
+
return ret;
 }
 
-- 
2.5.5

[PATCH 8/8] mwifiex: use better message and error code when OF node doesn't match

2016-05-27 Thread Javier Martinez Canillas

The Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt DT
binding document lists the possible compatible strings that a SDIO child
node can have, so the driver checks if the defined in the node matches.

But the error message when that's not the case is misleading, so change
for one that makes clear what the error really is. Also, returning a -1
as errno code is not correct since that's -EPERM. A -EINVAL seems to be
a more appropriate one.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 8b3292eaecb2..e6d56be04e08 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -103,8 +103,8 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
int ret;
 
if (!of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
-   dev_err(dev, "sdio platform data not available\n");
-   return -1;
+   dev_err(dev, "required compatible string missing\n");
+   return -EINVAL;
}
 
card->plt_of_node = dev->of_node;
-- 
2.5.5

[PATCH 5/8] mwifiex: use dev_err() instead of pr_err() in mwifiex_sdio_probe()

2016-05-27 Thread Javier Martinez Canillas

It's better to have the device name prefixed in the error message.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 7aeee88b858f..1ffbb972318f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -182,7 +182,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
sdio_release_host(func);
 
if (ret) {
-   pr_err("%s: failed to enable function\n", __func__);
+   dev_err(>dev, "failed to enable function\n");
goto err_free;
}
 
@@ -193,7 +193,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
ret = mwifiex_add_card(card, _remove_card_sem, _ops,
   MWIFIEX_SDIO);
if (ret) {
-   pr_err("%s: add card failed\n", __func__);
+   dev_err(>dev, "add card failed\n");
goto err_disable;
}
 
-- 
2.5.5

[PATCH 7/8] mwifiex: don't print an error if an optional DT property is missing

2016-05-27 Thread Javier Martinez Canillas

The Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt DT
binding document say that the "interrupts" property in the child node is
optional. So the property being missed shouldn't be treated as an error.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 1c17e624547a..8b3292eaecb2 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -114,7 +114,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
if (cfg && card->plt_of_node) {
cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
if (!cfg->irq_wifi) {
-   dev_err(dev,
+   dev_dbg(dev,
"fail to parse irq_wifi from device tree\n");
} else {
ret = devm_request_irq(dev, cfg->irq_wifi,
-- 
2.5.5

[PATCH 6/8] mwifiex: check if mwifiex_sdio_probe_of() fails and return error

2016-05-27 Thread Javier Martinez Canillas

The function can fail so the returned value should be checked
and the error propagated to the caller in case of a failure.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 1ffbb972318f..1c17e624547a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -187,8 +187,13 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
}
 
/* device tree node parsing and platform specific configuration*/
-   if (func->dev.of_node)
-   mwifiex_sdio_probe_of(>dev, card);
+   if (func->dev.of_node) {
+   ret = mwifiex_sdio_probe_of(>dev, card);
+   if (ret) {
+   dev_err(>dev, "SDIO dt node parse failed\n");
+   goto err_disable;
+   }
+   }
 
ret = mwifiex_add_card(card, _remove_card_sem, _ops,
   MWIFIEX_SDIO);
-- 
2.5.5

Re: [RFC] Reset pins of phys and their representation in a device tree

2016-05-12 Thread Javier Martinez Canillas

Hello ChenYu

On Thu, May 12, 2016 at 9:49 AM, Chen-Yu Tsai <w...@csie.org> wrote:
> Hi,
>
> On Thu, May 12, 2016 at 9:40 PM, Javier Martinez Canillas
> <jav...@dowhile0.org> wrote:
>> [adding Krzysztof as cc]
>>
>> On Thu, May 12, 2016 at 8:25 AM, Sergei Shtylyov
>> <sergei.shtyl...@cogentembedded.com> wrote:
>>> Hello.
>>>
>>>
>>> On 5/12/2016 10:15 AM, Uwe Kleine-König wrote:
>>>
>>>> I have a machine here where the reset pin of the phy is connected to a
>>>> GPIO.
>>>>
>>>> There are different possibilities available today to handle this
>>>> situation, here are the ones I'm aware of:
>>>>
>>>>  - Use a gpio-hog to set the reset gpio to non-active
>>>>This might result in dependency problems (and that's what I am
>>>>currently faced with) because there is no connection in the device
>>>>tree between the hog and the phy.
>>>>
>>>>  - [Documentation/devicetree/bindings/net/fsl-fec.txt]
>>>>The fec node supports properties
>>>>
>>>> phy-reset-gpios = < 14 0>;
>>>> phy-reset-duration = <200> /* milliseconds */;
>>>>
>>>>Something similar exists in TI's vendor kernel
>>>>
>>>> (http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/17d192b999ee904ced223c16cef76111a51c461b)
>>>>with different (and IMHO bader) naming.
>>>>This is the wrong place to specify the gpios; they shouldn't be in the
>>>>mac's node, but in a phy node instead.
>>>>
>>>> So what I actually want is to put the gpio specification in the right
>>>> place and let it look as follows:
>>>>
>>>> mymdiobus {
>>>> [...]
>>>> myfirstphy: ethernet-phy@0 {
>>>> compatible = "ethernet-phy-ieee802.3-c22";
>>>> reg = <0>;
>>>>
>>>> reset-gpios = < 14 0>;
>>>> reset-duration-ms = <200>;
>>>> };
>>>>
>>>> mysecondphy: ethernet-phy@2 {
>>>> compatible = "ethernet-phy-ieee802.3-c22";
>>>> reg = <2>;
>>>>
>>>> reset-gpios = < 10 0>;
>>>> reset-duration-ms = <200>;
>>>> };
>>>> };
>>>>
>>>> And with this we could defer probe of  if  isn't
>>>> available yet.
>>>>
>>>> Does this sound sensible? Does something like that already exist which I
>>>> missed? Any further ideas/comments?
>>>
>>>
>>> http://patchwork.ozlabs.org/patch/616495/
>>>
>>>I'll post a new version RSN.
>>>
>>
>> This seems to be similar to what's needed by MMC/SD/SDIO devices (i.e:
>> a WiFi chip) that needs some power sequence for reset and be
>> enumerable.
>>
>> Krzysztof has been working  to make the MMC pwrseq framework more
>> generic [0] since he wants to use it also for built-in USB devices.
>>
>> From a quick look at the patches mentioned in this thread, it seems
>> that the same framework can be used to reset the PHYs unless I'm
>> missing something. Have you considered using this? It would be good if
>> there is a consistent way to define the power sequence for devices
>> across the different subsystems.
>>
>> [0]: https://lkml.org/lkml/2016/5/5/230
>
> You probably missed the part that Rob nacked the bindings in that
> series?
>

No, I didn't miss that. AFAICT the way forward is still being
discussed as you can see from the last email in that thread from Ulf
[0].

And yes, the DT bindings is likely to change (keeping the backward
compat for MMC) which is good,  but still the power sequence
management code can be reused across subsystems.

> Putting the reset gpios under the PHY nodes and handling it from
> phylib is probably the way to go. I'd like to ask for regulator
> and clock support as well, if possible.
>

A GPIO array, clock and regulator (after Krzysztof patch) is already
supported by the pwrseq_simple provider for example.

> Regards
> ChenYu

[0]: https://lkml.org/lkml/2016/5/10/320

Re: [RFC] Reset pins of phys and their representation in a device tree

2016-05-12 Thread Javier Martinez Canillas

[adding Krzysztof as cc]

On Thu, May 12, 2016 at 8:25 AM, Sergei Shtylyov
 wrote:
> Hello.
>
>
> On 5/12/2016 10:15 AM, Uwe Kleine-König wrote:
>
>> I have a machine here where the reset pin of the phy is connected to a
>> GPIO.
>>
>> There are different possibilities available today to handle this
>> situation, here are the ones I'm aware of:
>>
>>  - Use a gpio-hog to set the reset gpio to non-active
>>This might result in dependency problems (and that's what I am
>>currently faced with) because there is no connection in the device
>>tree between the hog and the phy.
>>
>>  - [Documentation/devicetree/bindings/net/fsl-fec.txt]
>>The fec node supports properties
>>
>> phy-reset-gpios = < 14 0>;
>> phy-reset-duration = <200> /* milliseconds */;
>>
>>Something similar exists in TI's vendor kernel
>>
>> (http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/17d192b999ee904ced223c16cef76111a51c461b)
>>with different (and IMHO bader) naming.
>>This is the wrong place to specify the gpios; they shouldn't be in the
>>mac's node, but in a phy node instead.
>>
>> So what I actually want is to put the gpio specification in the right
>> place and let it look as follows:
>>
>> mymdiobus {
>> [...]
>> myfirstphy: ethernet-phy@0 {
>> compatible = "ethernet-phy-ieee802.3-c22";
>> reg = <0>;
>>
>> reset-gpios = < 14 0>;
>> reset-duration-ms = <200>;
>> };
>>
>> mysecondphy: ethernet-phy@2 {
>> compatible = "ethernet-phy-ieee802.3-c22";
>> reg = <2>;
>>
>> reset-gpios = < 10 0>;
>> reset-duration-ms = <200>;
>> };
>> };
>>
>> And with this we could defer probe of  if  isn't
>> available yet.
>>
>> Does this sound sensible? Does something like that already exist which I
>> missed? Any further ideas/comments?
>
>
> http://patchwork.ozlabs.org/patch/616495/
>
>I'll post a new version RSN.
>

This seems to be similar to what's needed by MMC/SD/SDIO devices (i.e:
a WiFi chip) that needs some power sequence for reset and be
enumerable.

Krzysztof has been working  to make the MMC pwrseq framework more
generic [0] since he wants to use it also for built-in USB devices.

>From a quick look at the patches mentioned in this thread, it seems
that the same framework can be used to reset the PHYs unless I'm
missing something. Have you considered using this? It would be good if
there is a consistent way to define the power sequence for devices
across the different subsystems.

[0]: https://lkml.org/lkml/2016/5/5/230

Best regards,
Javier

[PATCH 0/2] net: encx24j600: Fix SPI driver module autoload

2015-10-30 Thread Javier Martinez Canillas

Hello,

Recently I've been trying to fix module autoloading for all SPI drivers and
found that the encx24j600 driver does not fill module alias information due
missing a MODULE_DEVICE_TABLE() so module autload won't work and the driver
Kconfig symbol is tristate which means the driver can be built as a module.

But also the SPI id table is not correctly defined so this series fixes both
issues.

The patches have been applied on top of the next-next tree master branch:

git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master

Best regards,
Javier


Javier Martinez Canillas (2):
  net: encx24j600: Fix SPI id table definition
  net: encx24j600: Export missing SPI module alias information

 drivers/net/ethernet/microchip/encx24j600.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] net: encx24j600: Fix SPI id table definition

2015-10-30 Thread Javier Martinez Canillas

A driver's SPI id table is expected to be an array of struct spi_device_id
that ends with a zero-initialized sentinel entry. But this driver defines
the table as a single struct spi_device_id and sets .id_table to a pointer
to this struct.

But spi_match_id() has a loop that iterates while the struct spi_device_id
.name[0] is not NULL, so not having a sentinel can cause a NULL pointer
deference error.

This patch defines the SPI id table correctly as all other SPI drivers do.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

 drivers/net/ethernet/microchip/encx24j600.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microchip/encx24j600.c 
b/drivers/net/ethernet/microchip/encx24j600.c
index bf08ce2baf8d..c2dafc1c53de 100644
--- a/drivers/net/ethernet/microchip/encx24j600.c
+++ b/drivers/net/ethernet/microchip/encx24j600.c
@@ -1094,8 +1094,9 @@ static int encx24j600_spi_remove(struct spi_device *spi)
return 0;
 }
 
-static const struct spi_device_id encx24j600_spi_id_table = {
-   .name = "encx24j600"
+static const struct spi_device_id encx24j600_spi_id_table[] = {
+   { .name = "encx24j600" },
+   { /* sentinel */ }
 };
 
 static struct spi_driver encx24j600_spi_net_driver = {
@@ -1106,7 +1107,7 @@ static struct spi_driver encx24j600_spi_net_driver = {
},
.probe  = encx24j600_spi_probe,
.remove = encx24j600_spi_remove,
-   .id_table   = _spi_id_table,
+   .id_table   = encx24j600_spi_id_table,
 };
 
 static int __init encx24j600_init(void)
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] net: encx24j600: Export missing SPI module alias information

2015-10-30 Thread Javier Martinez Canillas

The driver Kconfig symbol is tristate which means that it can be built as
a module but the module alias information is not added to the module info
so module autoload won't work since user-space won't have the information.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/ethernet/microchip/encx24j600.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/microchip/encx24j600.c 
b/drivers/net/ethernet/microchip/encx24j600.c
index c2dafc1c53de..2056b719c262 100644
--- a/drivers/net/ethernet/microchip/encx24j600.c
+++ b/drivers/net/ethernet/microchip/encx24j600.c
@@ -1098,6 +1098,7 @@ static const struct spi_device_id 
encx24j600_spi_id_table[] = {
{ .name = "encx24j600" },
{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(spi, encx24j600_spi_id_table);
 
 static struct spi_driver encx24j600_spi_net_driver = {
.driver = {
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH] net: ks8851: Export OF module alias information

2015-09-16 Thread Javier Martinez Canillas

Drivers needs to export the OF id table and this be built into
the module or udev won't have the necessary information to autoload
the driver module when the device is registered via OF.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

 drivers/net/ethernet/micrel/ks8851.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/micrel/ks8851.c 
b/drivers/net/ethernet/micrel/ks8851.c
index 66d4ab703f45..60f43ec22175 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1601,6 +1601,7 @@ static const struct of_device_id ks8851_match_table[] = {
{ .compatible = "micrel,ks8851" },
{ }
 };
+MODULE_DEVICE_TABLE(of, ks8851_match_table);
 
 static struct spi_driver ks8851_driver = {
.driver = {
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/18] Export SPI and OF module aliases in missing drivers

2015-08-20 Thread Javier Martinez Canillas

Hello,

Short version:

This patch series is the SPI equivalent of the I2C one posted before [0].

This series add the missing MODULE_DEVICE_TABLE() for OF and SPI tables
to export that information so modules have the correct aliases built-in
and autoloading works correctly.

Longer version:

The SPI core always reports the MODALIAS uevent as spi:modalias
regardless of the mechanism that was used to register the device (i.e:
OF or board code) and the table that is used later to match the driver
with the device (i.e: SPI id table or OF match table).

But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the device node's compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.

Also, there are many SPI drivers whose module auto-loading is not working
because of this fact that the SPI core always reports the MODALIAS as
spi:modalias and many developers didn't expect this since is not how
other subsystems behave.

I've identified SPI drivers with 3 types of different issues:

a) Those that have an spi_table but are not exported. The match works
   if the driver is built-in but since the ID table is not exported,
   module auto-load won't work.

b) Those that have a of_table but are not exported. This is currently
   not an issue since even when the of_table is used to match the dev
   with the driver, an OF modalias is not reported by the SPI core.
   But if the SPI core is changed to report the MODALIAS of the form
   of:N*T*C as it's made by other subsystems, then module auto-load
   will break for these drivers.

c) Those that don't have an of_table but should since are OF drivers
   with DT bindings doc for them. Since the SPI core does not report
   a OF modalias and since spi_match_device() fallbacks to match the
   device part of the compatible string with the SPI device ID table,
   many OF drivers don't have an of_table to match. After all having
   a SPI device ID table is mandatory so it works without a of_table.

So, in order to not make mandatory to have a SPI device ID table, all
these three kind of issues have to be addressed. This series does that.

I split the changes so the patches in this series are independent and
can be picked individually by subsystem maintainers.

Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches

Patch #18 changes the logic of spi_uevent() to report an OF modalias if
the device was registered using OF. But this patch is included in the
series only as an RFC for illustration purposes since changing that
without first applying all the other patches in this series, will break
module autoloading for the drivers of devices registered using OF but
that lacks an of_match_table. I'll repost patch #18 once all the patches
in this series have landed.

[0]: https://lkml.org/lkml/2015/7/30/519

Best regards,
Javier


Javier Martinez Canillas (18):
  iio: Export SPI module alias information in missing drivers
  staging: iio: hmc5843: Export missing SPI module alias information
  mtd: dataflash: Export OF module alias information
  OMAPDSS: panel-sony-acx565akm: Export OF module alias information
  mmc: mmc_spi: Export OF module alias information
  staging: mt29f_spinand: Export OF module alias information
  net: ks8851: Export OF module alias information
  [media] s5c73m3: Export OF module alias information
  mfd: cros_ec: spi: Add OF match table
  iio: dac: ad7303: Add OF match table
  iio: adc: max1027: Set struct spi_driver .of_match_table
  mfd: stmpe: Add OF match table
  iio: adc: mcp320x: Set struct spi_driver .of_match_table
  iio: as3935: Add OF match table
  iio: adc128s052: Add OF match table
  iio: frequency: adf4350: Add OF match table
  NFC: trf7970a: Add OF match table
  spi: (RFC, don't apply) report OF style modalias when probing using DT

 drivers/iio/adc/max1027.c   |  1 +
 drivers/iio/adc/mcp320x.c   |  1 +
 drivers/iio/adc/ti-adc128s052.c |  8 
 drivers/iio/amplifiers/ad8366.c |  1 +
 drivers/iio/dac/ad7303.c|  7 +++
 drivers/iio/frequency/adf4350.c |  9 +
 drivers/iio/proximity/as3935.c  |  7 +++
 drivers/media/i2c/s5c73m3/s5c73m3-spi.c |  1 +
 drivers/mfd/cros_ec_spi.c   |  7 +++
 drivers/mfd/stmpe-spi.c | 13 +
 drivers/mmc/host/mmc_spi.c  |  1 +
 drivers/mtd/devices/mtd_dataflash.c |  1 +
 drivers/net/ethernet/micrel/ks8851.c|  1 +
 drivers/nfc/trf7970a.c  |  7 +++
 drivers/spi/spi.c   |  8

[PATCH 07/18] net: ks8851: Export OF module alias information

2015-08-20 Thread Javier Martinez Canillas

The SPI core always reports the MODALIAS uevent as spi:modalias
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).

So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.

But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.

To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.

Signed-off-by: Javier Martinez Canillas jav...@osg.samsung.com
---

 drivers/net/ethernet/micrel/ks8851.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/micrel/ks8851.c 
b/drivers/net/ethernet/micrel/ks8851.c
index 66d4ab703f45..60f43ec22175 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1601,6 +1601,7 @@ static const struct of_device_id ks8851_match_table[] = {
{ .compatible = micrel,ks8851 },
{ }
 };
+MODULE_DEVICE_TABLE(of, ks8851_match_table);
 
 static struct spi_driver ks8851_driver = {
.driver = {
-- 
2.4.3

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

75 matches

Mail list logo