Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers/atwinc15x0: add Kconfig #17382

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
+480 −16
Open

drivers/atwinc15x0: add Kconfig #17382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ff37253
pkg/driver_atwinc15x0: fix dependendies
gschorcht Dec 11, 2021
503c94a
drivers/atwinc15x0: add Kconfig
gschorcht Dec 11, 2021
6befb7d
tests/driver_netdev_common: add Kconfig
gschorcht Dec 11, 2021
047c60a
tests/driver_atwinc15x0: add Kconfig
gschorcht Dec 11, 2021
14a3792
fixup! drivers/atwinc15x0: add Kconfig
gschorcht Dec 12, 2021
e21f575
fixup! drivers/atwinc15x0: add Kconfig
gschorcht Dec 12, 2021
4091611
fixup! drivers/atwinc15x0: add Kconfig
gschorcht Dec 12, 2021
57fc6f8
fixup! drivers/atwinc15x0: add Kconfig
gschorcht Dec 24, 2021
9c4ee95
fixup! tests/driver_netdev_common: add Kconfig
gschorcht Dec 24, 2021
bac0efb
fixup! tests/driver_atwinc15x0: add Kconfig
gschorcht Dec 24, 2021
de96ed6
fixup! drivers/atwinc15x0: add Kconfig
gschorcht Dec 27, 2021
29f8ab6
drivers/atwinc15x0: small doc improvements
gschorcht Dec 27, 2021
c533b6c
REMOVEME
gschorcht Dec 26, 2021
ae17fcc
REMOVEME
gschorcht Dec 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpu/nrf5x_common/radio/Kconfig.nrf5x
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

config HAVE_NRF5X_RADIO
bool
select NRF5X_RADIO if MODULE_NETDEV_DEFAULT
select NRF5X_RADIO if MODULE_NETDEV_DEFAULT && !PACKAGE_NIMBLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjmolinas can you check if this is needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nimble has not been modeled so far, but what should be done is change the default NRF5X_RADIO backend if using Nimble no? I think something like this:

diff --git a/cpu/nrf5x_common/radio/Kconfig.nrf5x b/cpu/nrf5x_common/radio/Kconfig.nrf5x
index c98a0059b6..05ca54067b 100644
--- a/cpu/nrf5x_common/radio/Kconfig.nrf5x
+++ b/cpu/nrf5x_common/radio/Kconfig.nrf5x
@@ -22,6 +22,7 @@ if NRF5X_RADIO

 choice NRF5X_RADIO_BACKEND
     bool "nrf5x radio backend"
+    default MODULE_NRFBLE if PACKAGE_NIMBLE

 config MODULE_NRF802154
     bool "Implementation of the IEEE 802.15.4 for nRF52 radio"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If when using nimble the backend is fixed, then we should also hide the prompt, so it cannot be changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True

help
Indicates that an NRF5X radio is present.

Expand Down
1 change: 1 addition & 0 deletions drivers/Kconfig.net
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
menu "Network Device Drivers"
rsource "at86rf215/Kconfig"
rsource "ata8520e/Kconfig"
rsource "atwinc15x0/Kconfig"
rsource "can_trx/Kconfig"
rsource "cc110x/Kconfig"
rsource "$(RIOTCPU)/cc2538/radio/Kconfig"
Expand Down
58 changes: 58 additions & 0 deletions drivers/atwinc15x0/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Copyright (c) 2020 HAW Hamburg
#
# This file is subject to the terms and conditions of the GNU Lesser
# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.
#

menuconfig MODULE_ATWINC15X0
bool
prompt "ATWINC15x0 WiFi module" if !(MODULE_NETDEV_DEFAULT && HAVE_ATWINC15X0)
default y if (MODULE_NETDEV_DEFAULT && HAVE_ATWINC15X0)
depends on HAS_PERIPH_GPIO
depends on HAS_PERIPH_GPIO_IRQ
depends on HAS_PERIPH_SPI
depends on TEST_KCONFIG
select PACKAGE_DRIVER_ATWINC15X0
select MODULE_PERIPH_GPIO
select MODULE_PERIPH_GPIO_IRQ
select MODULE_PERIPH_SPI
select MODULE_NETDEV_ETH
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but I think this is not modelled yet. That's the main reason we did not move yet to modelling networking drivers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drivers/net/netdev seems not to be so much complicated. Since there were no error messages, maybe we should keep it so it won't be forgotten later?

select MODULE_ZTIMER_MSEC
gschorcht marked this conversation as resolved.
Show resolved Hide resolved
help
This netdev device driver is used for the Microchip ATWINC15x0
WiFi module. The ATWINC15x0 WiFi module is widely used as WiFi
interface on various boards and extension boards.

if MODULE_ATWINC15X0

config WIFI_SSID
string "WiFi SSID"
default "RIOT_AP"
help
SSID of the AP to be used. The SSID must not contain more
than 32 characters.

config WIFI_USE_WPA2
bool "Use WPA2 authentication"
default n
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default n is the default behaviour, no need to specify it.

help
Specify whether WPA2 authentication is used to connect to the
AP. If WPA2 authentication is enabled, the passphrase has to be
defined.

config WIFI_PASS
string "Passphrase for WPA2 Personal Mode authentication"
depends on WIFI_USE_WPA2
default "This is RIOT-OS"
help
The passphrase is defined as a clear text string with a maximum
of 64 characters. It is used for WPA2 personal mode authentication.
Comment on lines +29 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely outside the scope of this PR, but at some point we need to handle this system-wide and not driver-specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this seems to be a hen's egg problem because it is required for the compilation of the driver.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as it is currently done. Perhaps we can think about some key management (something similar to credman?). Would it be possible (on a separate PR) to start unifying the macro name and have one system-wide for other modules (e.g., ESP)?


endif # MODULE_ATWINC15X0

config HAVE_ATWINC15X0
bool
help
Indicates that a ATWINC15x0 module is present.

11 changes: 7 additions & 4 deletions drivers/atwinc15x0/doc.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,13 @@ Symbol | Default | Description

At the moment only WPA2 Personal Mode is supported. The required settings are:

Parameter | Default | Description
:---------|:----------|:------------
WIFI_SSID | "RIOT_AP" | SSID of the AP to be used.
WIFI_PASS | - | Passphrase used for the AP as clear text (max. 64 chars).
Parameter | Default | Description
:----------|:----------|:------------
#WIFI_SSID | "RIOT_AP" | SSID of the AP to be used.
#WIFI_PASS | - | Passphrase used for the AP as clear text (max. 64 chars) [1].
Comment on lines +161 to +162
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to migrate these values to include the CONFIG_ prefix

Copy link
Contributor Author

@gschorcht gschorcht Jan 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that user applications may have already used these documented configuration settings. We can change them but we should have some deprecation mechanism which. My approach was to use it in that way

#ifndef WIFI_SSID
#ifndef CONFIG_WIFI_SSID
#define WIFI_SSID       "RIOT_AP"
#else /* CONFIG_WIFI_SSID */
#define WIFI_SSID       CONFIG_WIFI_SSID
#endif /* CONFIG_WIFI_SSID */
#endif /* WIFI_SSID */

which allows to use both definitions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we should add a deprecation warning if we change them. It already has been done with other configurations (e.g., #13129).



1. Leave `WIFI_PASS` undefined to connect to an open WiFi access point.

The following example shows the make command with the setting of different GPIOs and the WiFi parameters.
```
Expand Down
20 changes: 17 additions & 3 deletions drivers/atwinc15x0/include/atwinc15x0_params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,30 @@ extern "C" {

/**
* @brief SSID of the AP to be used.
*
* The SSID must not contain more than 32 characters.
*/
#ifndef WIFI_SSID
#ifndef CONFIG_WIFI_SSID
#define WIFI_SSID "RIOT_AP"
#endif
#else /* CONFIG_WIFI_SSID */
#define WIFI_SSID CONFIG_WIFI_SSID
#endif /* CONFIG_WIFI_SSID */
#endif /* WIFI_SSID */

/**
* @brief Passphrase used for the AP as clear text (max. 64 chars).
*
* The passphrase is defined as a clear text string with a maximum
* of 64 characters. It is used for WPA2 personal mode authentication.
*
* If passphrase is not defined, an "open" AP without WPA2 authentication is
* used.
*/
#ifdef DOXYGEN
#define WIFI_PASS "ThisistheRIOTporttoESP"
#if DOXYGEN
#define WIFI_PASS "This is RIOT-OS"
#elif defined(CONFIG_WIFI_PASS)
#define WIFI_PASS CONFIG_WIFI_PASS
#endif

/**
Expand Down
11 changes: 4 additions & 7 deletions pkg/driver_atwinc15x0/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@
#

config PACKAGE_DRIVER_ATWINC15X0
bool "ATWINC15x0 WiFi driver package"
bool
depends on TEST_KCONFIG
depends on HAS_PERIPH_SPI
depends on HAS_PERIPH_GPIO
depends on HAS_PERIPH_GPIO_IRQ
select MODULE_PERIPH_SPI
select MODULE_PERIPH_GPIO
select MODULE_PERIPH_GPIO_IRQ
depends on MODULE_ATWINC15X0
gschorcht marked this conversation as resolved.
Show resolved Hide resolved
select MODULE_DRIVER_ATWINC15X0
select MODULE_DRIVER_ATWINC15X0_COMMON
select MODULE_DRIVER_ATWINC15X0_SPI_FLASH
help
ATWINC15x0 WiFi vendor driver from the Arduino WiFi101 library.
This is needed by the RIOT ATWINC15x0 driver.

config MODULE_DRIVER_ATWINC15X0
bool
Expand All @@ -30,3 +26,4 @@ config MODULE_DRIVER_ATWINC15X0_COMMON

config MODULE_DRIVER_ATWINC15X0_SPI_FLASH
bool
depends on TEST_KCONFIG
2 changes: 1 addition & 1 deletion sys/random/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ if MODULE_RANDOM
choice RANDOM_IMPLEMENTATION
bool "PRNG Implementation"
depends on TEST_KCONFIG
default MODULE_PRNG_HWRNG if HAS_PERIPH_HWRNG
default MODULE_PRNG_HWRNG if HAS_PERIPH_HWRNG && !MODULE_NETDEV_DEFAULT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I don't remember but there was a problem when module netdev_default is used.

default MODULE_PRNG_MUSL_LCG

menuconfig MODULE_PRNG_FORTUNA
Expand Down
8 changes: 8 additions & 0 deletions tests/driver_atwinc15x0/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Copyright (c) 2021 Gunar Schorcht
#
# This file is subject to the terms and conditions of the GNU Lesser
# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.
#

rsource "../driver_netdev_common/Kconfig"
4 changes: 4 additions & 0 deletions tests/driver_atwinc15x0/app.config.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# this file enables modules defined in Kconfig. Do not use this file for
# application configuration. This is only needed during migration.
CONFIG_DRIVER_NETDEV_COMMON=y
CONFIG_MODULE_ATWINC15X0=y
Loading