On 1 June 2017 at 22:57, John Stultz [off-list ref] wrote:
On Wed, May 31, 2017 at 11:36 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On 31/05/2017 20:14, John Stultz wrote:
quoted
On Mon, May 8, 2017 at 9:21 AM, Ulf Hansson [off-list ref] wrote:
quoted
The description of the connection between the dwmmc (SDIO) controller and
the Wifi chip, which is attached to the SDIO bus is wrong. Currently the
SDIO card can't be detected and thus the Wifi doesn't work.
Let's fix this by assigning the correct vmmc supply, which is the always on
regulator VDD_3V3 and remove the WLAN enable regulator altogether. Then to
properly deal with the power on/off sequence, add a mmc-pwrseq node to
describe the resources needed to detect the SDIO card.
Except for the WLAN enable GPIO and its corresponding assert/de-assert
delays, the mmc-pwrseq node also contains a handle to a clock provided by
the hi655x pmic. This clock is also needed to be able to turn on the WiFi
chip.
Signed-off-by: Ulf Hansson <redacted>
Ulf,
So oddly, this patch seems to have broken wifi on HiKey when it
landed in 4.12-rc3. I checked my config and CONFIG_PWRSEQ_SIMPLE is
enabled.
Hi John,
how is it possible the WiFi stops working with this series? This series
provides the missing bits to enable the WiFi with a vanilla kernel.
May be the WiFi is reset with this kernel and when it starts again the
firmware fails to load because it is not up-to-date?
Can you check in dmesg if there is a firmware error message?
So, it seems to me to be connected to some of the tweaks to the mmc2 dts node.
When it works I see:
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
mmc_host mmc2: card is non-removable.
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 400000Hz,
actual 400000HZ div = 31)
dwmmc_k3 f723f000.dwmmc2: 1 slots initialized
dwmmc_k3 f723f000.dwmmc2: card claims to support voltages below defined range
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 25000000Hz,
actual 24800000HZ div = 0)
mmc2: new SDIO card at address 0001
wl18xx_driver wl18xx.4.auto: Direct firmware load for
ti-connectivity/wl18xx-conf.bin failed with error -2
wl18xx_driver wl18xx.4.auto: Falling back to user helper
When it fails:
dwmmc_k3 f723f000.dwmmc2: fifo-depth property not found, using value
of FIFOTH register as default
dwmmc_k3 f723f000.dwmmc2: IDMAC supports 32-bit address mode.
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
is seen repeatedly.
I'm using my branch here (using the hikey_defconfig included):
https://git.linaro.org/people/john.stultz/android-dev.git dev/hikey-mainline-WIP
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
With the last patch included or removed to generate the working and
broken dmesgs respectively.
thanks
-john
From: John Stultz <hidden> Date: 2017-06-05 17:32:16
On Mon, Jun 5, 2017 at 8:15 AM, Ulf Hansson [off-list ref] wrote:
On 1 June 2017 at 22:57, John Stultz [off-list ref] wrote:
quoted
On Wed, May 31, 2017 at 11:36 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On 31/05/2017 20:14, John Stultz wrote:
quoted
On Mon, May 8, 2017 at 9:21 AM, Ulf Hansson [off-list ref] wrote:
quoted
The description of the connection between the dwmmc (SDIO) controller and
the Wifi chip, which is attached to the SDIO bus is wrong. Currently the
SDIO card can't be detected and thus the Wifi doesn't work.
Let's fix this by assigning the correct vmmc supply, which is the always on
regulator VDD_3V3 and remove the WLAN enable regulator altogether. Then to
properly deal with the power on/off sequence, add a mmc-pwrseq node to
describe the resources needed to detect the SDIO card.
Except for the WLAN enable GPIO and its corresponding assert/de-assert
delays, the mmc-pwrseq node also contains a handle to a clock provided by
the hi655x pmic. This clock is also needed to be able to turn on the WiFi
chip.
Signed-off-by: Ulf Hansson <redacted>
Ulf,
So oddly, this patch seems to have broken wifi on HiKey when it
landed in 4.12-rc3. I checked my config and CONFIG_PWRSEQ_SIMPLE is
enabled.
Hi John,
how is it possible the WiFi stops working with this series? This series
provides the missing bits to enable the WiFi with a vanilla kernel.
May be the WiFi is reset with this kernel and when it starts again the
firmware fails to load because it is not up-to-date?
Can you check in dmesg if there is a firmware error message?
So, it seems to me to be connected to some of the tweaks to the mmc2 dts node.
When it works I see:
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
mmc_host mmc2: card is non-removable.
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 400000Hz,
actual 400000HZ div = 31)
dwmmc_k3 f723f000.dwmmc2: 1 slots initialized
dwmmc_k3 f723f000.dwmmc2: card claims to support voltages below defined range
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 25000000Hz,
actual 24800000HZ div = 0)
mmc2: new SDIO card at address 0001
wl18xx_driver wl18xx.4.auto: Direct firmware load for
ti-connectivity/wl18xx-conf.bin failed with error -2
wl18xx_driver wl18xx.4.auto: Falling back to user helper
When it fails:
dwmmc_k3 f723f000.dwmmc2: fifo-depth property not found, using value
of FIFOTH register as default
dwmmc_k3 f723f000.dwmmc2: IDMAC supports 32-bit address mode.
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
is seen repeatedly.
I'm using my branch here (using the hikey_defconfig included):
https://git.linaro.org/people/john.stultz/android-dev.git dev/hikey-mainline-WIP
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Ah! Thanks for this!
Would it make sense to have the hi6220 clk select this if its
required? I had thought the Hi655x was a different SoC, not just a
PMIC. Expecting users to be able to figure any of this out doesn't
seem reasonable.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
Yep. With that it seems to work alright. I'll let you know if I see
any further problems as I continue to use it.
Thanks again for the help sorting this out!
-john
From: Daniel Lezcano <hidden> Date: 2017-06-05 18:14:02
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
---
arch/arm64/Kconfig.platforms | 1 +
1 file changed, 1 insertion(+)
From: John Stultz <hidden> Date: 2017-06-05 21:10:23
On Mon, Jun 5, 2017 at 8:15 AM, Ulf Hansson [off-list ref] wrote:
On 1 June 2017 at 22:57, John Stultz [off-list ref] wrote:
quoted
On Wed, May 31, 2017 at 11:36 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On 31/05/2017 20:14, John Stultz wrote:
quoted
On Mon, May 8, 2017 at 9:21 AM, Ulf Hansson [off-list ref] wrote:
quoted
The description of the connection between the dwmmc (SDIO) controller and
the Wifi chip, which is attached to the SDIO bus is wrong. Currently the
SDIO card can't be detected and thus the Wifi doesn't work.
Let's fix this by assigning the correct vmmc supply, which is the always on
regulator VDD_3V3 and remove the WLAN enable regulator altogether. Then to
properly deal with the power on/off sequence, add a mmc-pwrseq node to
describe the resources needed to detect the SDIO card.
Except for the WLAN enable GPIO and its corresponding assert/de-assert
delays, the mmc-pwrseq node also contains a handle to a clock provided by
the hi655x pmic. This clock is also needed to be able to turn on the WiFi
chip.
Signed-off-by: Ulf Hansson <redacted>
Ulf,
So oddly, this patch seems to have broken wifi on HiKey when it
landed in 4.12-rc3. I checked my config and CONFIG_PWRSEQ_SIMPLE is
enabled.
Hi John,
how is it possible the WiFi stops working with this series? This series
provides the missing bits to enable the WiFi with a vanilla kernel.
May be the WiFi is reset with this kernel and when it starts again the
firmware fails to load because it is not up-to-date?
Can you check in dmesg if there is a firmware error message?
So, it seems to me to be connected to some of the tweaks to the mmc2 dts node.
When it works I see:
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
mmc_host mmc2: card is non-removable.
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 400000Hz,
actual 400000HZ div = 31)
dwmmc_k3 f723f000.dwmmc2: 1 slots initialized
dwmmc_k3 f723f000.dwmmc2: card claims to support voltages below defined range
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 25000000Hz,
actual 24800000HZ div = 0)
mmc2: new SDIO card at address 0001
wl18xx_driver wl18xx.4.auto: Direct firmware load for
ti-connectivity/wl18xx-conf.bin failed with error -2
wl18xx_driver wl18xx.4.auto: Falling back to user helper
When it fails:
dwmmc_k3 f723f000.dwmmc2: fifo-depth property not found, using value
of FIFOTH register as default
dwmmc_k3 f723f000.dwmmc2: IDMAC supports 32-bit address mode.
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
is seen repeatedly.
I'm using my branch here (using the hikey_defconfig included):
https://git.linaro.org/people/john.stultz/android-dev.git dev/hikey-mainline-WIP
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
thanks
-john
From: Rob Herring <hidden> Date: 2017-06-05 21:29:40
On Mon, Jun 5, 2017 at 4:10 PM, John Stultz [off-list ref] wrote:
On Mon, Jun 5, 2017 at 8:15 AM, Ulf Hansson [off-list ref] wrote:
quoted
On 1 June 2017 at 22:57, John Stultz [off-list ref] wrote:
quoted
On Wed, May 31, 2017 at 11:36 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On 31/05/2017 20:14, John Stultz wrote:
quoted
On Mon, May 8, 2017 at 9:21 AM, Ulf Hansson [off-list ref] wrote:
quoted
The description of the connection between the dwmmc (SDIO) controller and
the Wifi chip, which is attached to the SDIO bus is wrong. Currently the
SDIO card can't be detected and thus the Wifi doesn't work.
Let's fix this by assigning the correct vmmc supply, which is the always on
regulator VDD_3V3 and remove the WLAN enable regulator altogether. Then to
properly deal with the power on/off sequence, add a mmc-pwrseq node to
describe the resources needed to detect the SDIO card.
Except for the WLAN enable GPIO and its corresponding assert/de-assert
delays, the mmc-pwrseq node also contains a handle to a clock provided by
the hi655x pmic. This clock is also needed to be able to turn on the WiFi
chip.
Signed-off-by: Ulf Hansson <redacted>
Ulf,
So oddly, this patch seems to have broken wifi on HiKey when it
landed in 4.12-rc3. I checked my config and CONFIG_PWRSEQ_SIMPLE is
enabled.
Hi John,
how is it possible the WiFi stops working with this series? This series
provides the missing bits to enable the WiFi with a vanilla kernel.
May be the WiFi is reset with this kernel and when it starts again the
firmware fails to load because it is not up-to-date?
Can you check in dmesg if there is a firmware error message?
So, it seems to me to be connected to some of the tweaks to the mmc2 dts node.
When it works I see:
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
mmc_host mmc2: card is non-removable.
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 400000Hz,
actual 400000HZ div = 31)
dwmmc_k3 f723f000.dwmmc2: 1 slots initialized
dwmmc_k3 f723f000.dwmmc2: card claims to support voltages below defined range
mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot req 25000000Hz,
actual 24800000HZ div = 0)
mmc2: new SDIO card at address 0001
wl18xx_driver wl18xx.4.auto: Direct firmware load for
ti-connectivity/wl18xx-conf.bin failed with error -2
wl18xx_driver wl18xx.4.auto: Falling back to user helper
When it fails:
dwmmc_k3 f723f000.dwmmc2: fifo-depth property not found, using value
of FIFOTH register as default
dwmmc_k3 f723f000.dwmmc2: IDMAC supports 32-bit address mode.
dwmmc_k3 f723f000.dwmmc2: Using internal DMA controller.
dwmmc_k3 f723f000.dwmmc2: Version ID is 250a
dwmmc_k3 f723f000.dwmmc2: DW MMC controller at irq 43,32 bit host
data width,128 deep fifo
is seen repeatedly.
I'm using my branch here (using the hikey_defconfig included):
https://git.linaro.org/people/john.stultz/android-dev.git dev/hikey-mainline-WIP
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
As Rob said below, this is because the blue-tooth driver doesn't
properly deal with the power on/off sequence.
I guess it works for you because your versions of the boot binaries
turns all needed resources on. That isn't the case for me, blue-tooth
is neither working before or after this change, but wifi is.
quoted
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
Since the issues you are reporting about is depending on the
boot-binaries and not a really bugs in the kernel, may I suggest that
we invest our efforts in fixing the blue-tooth driver instead, as it's
there the real problem is!?
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
Correct.
I am working on patch, I keep you on cc.
Kind regards
Uffe
On 6 June 2017 at 12:08, Ulf Hansson [off-list ref] wrote:
[...]
quoted
quoted
quoted
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
As Rob said below, this is because the blue-tooth driver doesn't
properly deal with the power on/off sequence.
I guess it works for you because your versions of the boot binaries
turns all needed resources on. That isn't the case for me, blue-tooth
is neither working before or after this change, but wifi is.
quoted
quoted
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
Since the issues you are reporting about is depending on the
boot-binaries and not a really bugs in the kernel, may I suggest that
we invest our efforts in fixing the blue-tooth driver instead, as it's
there the real problem is!?
quoted
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
Correct.
I am working on patch, I keep you on cc.
Rob, John,
I have now looked into this a bit more. So I have some local patches,
which in principle adds the external clock to the bluetooth node for
the hikey dts, and adapts the uart bluetooth driver
(drivers/bluetooth/hci_ll.c) to also take the ext clock into
consideration while powering on/off the chip. I am working on a
vanilla kernel, thus not John's tree.
However, I can't get the uart1 amba device to be added because of this
error during boot:
"OF: amba_device_add() failed (-19) for /soc/uart at f7111000"
The reason why it fails is because amba_device_try_add() fails to read
the amba periphid of the uart1 device. I haven't yet been able to
figure out why.
Is this problem something you have seen before? I tried out v4.11,
v4.12-rc3 and John's tree (dev/hikey-mainline-WIP which is based upon
4.12.rc3), they all suffer from the same problem, not being able to
add the uart1 device.
The consequence then becomes that the bluetooth node (which is a child
node for the uart1 node), added in the below commit by Rob, never gets
parsed and thus the device don't become added. In other words, I
haven't been able to test my changes since I can't even get the
bluetooth device to be added.
John, are you using the pcm bluetooth interface or the uart?
commit 019aa56b7df8a796b2c01a56269a370ad3442ec7
Author: Rob Herring [off-list ref]
Date: Thu Apr 13 10:03:53 2017 -0500
arm64: dts: hikey: add WL1835 Bluetooth device node
This adds the serial slave device for the WL1835 Bluetooth interface.
Signed-off-by: Rob Herring [off-list ref]
Cc: Wei Xu [off-list ref]
Cc: Mark Rutland [off-list ref]
Signed-off-by: Marcel Holtmann [off-list ref]
Kind regards
Uffe
On 5 June 2017 at 20:13, Daniel Lezcano [off-list ref] wrote:
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
Acked-by: Ulf Hansson <redacted>
Would be nice to get this trivial fix in for 4.12 rcs to make the WiFi
work for Hikey.
Kind regards
Uffe
From: John Stultz <hidden> Date: 2017-06-06 15:58:30
On Tue, Jun 6, 2017 at 3:08 AM, Ulf Hansson [off-list ref] wrote:
[...]
quoted
quoted
quoted
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
As Rob said below, this is because the blue-tooth driver doesn't
properly deal with the power on/off sequence.
I guess it works for you because your versions of the boot binaries
turns all needed resources on. That isn't the case for me, blue-tooth
is neither working before or after this change, but wifi is.
quoted
quoted
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
Since the issues you are reporting about is depending on the
boot-binaries and not a really bugs in the kernel, may I suggest that
we invest our efforts in fixing the blue-tooth driver instead, as it's
there the real problem is!?
I'd agree a fast fix to the bluetooth node would be preferred. But to
the issue of bugs in the kernel vs bootloader, to users it doesn't
matter. It worked before, and now it doesn't. That's a regression, so
either we need a fix or a revert.
quoted
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
From: John Stultz <hidden> Date: 2017-06-06 16:24:38
On Tue, Jun 6, 2017 at 7:13 AM, Ulf Hansson [off-list ref] wrote:
On 6 June 2017 at 12:08, Ulf Hansson [off-list ref] wrote:
quoted
[...]
quoted
quoted
quoted
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
As Rob said below, this is because the blue-tooth driver doesn't
properly deal with the power on/off sequence.
I guess it works for you because your versions of the boot binaries
turns all needed resources on. That isn't the case for me, blue-tooth
is neither working before or after this change, but wifi is.
quoted
quoted
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
Since the issues you are reporting about is depending on the
boot-binaries and not a really bugs in the kernel, may I suggest that
we invest our efforts in fixing the blue-tooth driver instead, as it's
there the real problem is!?
quoted
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
Correct.
I am working on patch, I keep you on cc.
Rob, John,
I have now looked into this a bit more. So I have some local patches,
which in principle adds the external clock to the bluetooth node for
the hikey dts, and adapts the uart bluetooth driver
(drivers/bluetooth/hci_ll.c) to also take the ext clock into
consideration while powering on/off the chip. I am working on a
vanilla kernel, thus not John's tree.
However, I can't get the uart1 amba device to be added because of this
error during boot:
"OF: amba_device_add() failed (-19) for /soc/uart at f7111000"
The reason why it fails is because amba_device_try_add() fails to read
the amba periphid of the uart1 device. I haven't yet been able to
figure out why.
Is this problem something you have seen before? I tried out v4.11,
v4.12-rc3 and John's tree (dev/hikey-mainline-WIP which is based upon
4.12.rc3), they all suffer from the same problem, not being able to
add the uart1 device.
The consequence then becomes that the bluetooth node (which is a child
node for the uart1 node), added in the below commit by Rob, never gets
parsed and thus the device don't become added. In other words, I
haven't been able to test my changes since I can't even get the
bluetooth device to be added.
John, are you using the pcm bluetooth interface or the uart?
UART.
I'm not sure why initializing the UART fails for you. I suspect again
it might be related to differences in the bootloader, as I'm not sure
if uboot has had nearly the amount of usage as UEFI.
If you want to send a patch my way, I'm happy to test it, or you can
grab debian images that use UEFI here:
https://www.96boards.org/documentation/ConsumerEdition/HiKey/Downloads/Debian.md/
thanks
-john
On 6 June 2017 at 18:24, John Stultz [off-list ref] wrote:
On Tue, Jun 6, 2017 at 7:13 AM, Ulf Hansson [off-list ref] wrote:
quoted
On 6 June 2017 at 12:08, Ulf Hansson [off-list ref] wrote:
quoted
[...]
quoted
quoted
quoted
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
As Rob said below, this is because the blue-tooth driver doesn't
properly deal with the power on/off sequence.
I guess it works for you because your versions of the boot binaries
turns all needed resources on. That isn't the case for me, blue-tooth
is neither working before or after this change, but wifi is.
quoted
quoted
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
Since the issues you are reporting about is depending on the
boot-binaries and not a really bugs in the kernel, may I suggest that
we invest our efforts in fixing the blue-tooth driver instead, as it's
there the real problem is!?
quoted
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
Correct.
I am working on patch, I keep you on cc.
Rob, John,
I have now looked into this a bit more. So I have some local patches,
which in principle adds the external clock to the bluetooth node for
the hikey dts, and adapts the uart bluetooth driver
(drivers/bluetooth/hci_ll.c) to also take the ext clock into
consideration while powering on/off the chip. I am working on a
vanilla kernel, thus not John's tree.
However, I can't get the uart1 amba device to be added because of this
error during boot:
"OF: amba_device_add() failed (-19) for /soc/uart at f7111000"
The reason why it fails is because amba_device_try_add() fails to read
the amba periphid of the uart1 device. I haven't yet been able to
figure out why.
Is this problem something you have seen before? I tried out v4.11,
v4.12-rc3 and John's tree (dev/hikey-mainline-WIP which is based upon
4.12.rc3), they all suffer from the same problem, not being able to
add the uart1 device.
The consequence then becomes that the bluetooth node (which is a child
node for the uart1 node), added in the below commit by Rob, never gets
parsed and thus the device don't become added. In other words, I
haven't been able to test my changes since I can't even get the
bluetooth device to be added.
John, are you using the pcm bluetooth interface or the uart?
UART.
I'm not sure why initializing the UART fails for you. I suspect again
it might be related to differences in the bootloader, as I'm not sure
if uboot has had nearly the amount of usage as UEFI.
You are probably right, but that also makes me worried, as we have
likely other similar issues where UEFI just magically solves things
for the kernel.
Seems like I need to give UEFI a try again, however this time I would
really appreciate your help in testing as I am running out of
bandwidth for this task.
Just about to post the patches....
From: John Stultz <hidden> Date: 2017-06-07 05:25:47
On Tue, Jun 6, 2017 at 9:24 PM, Ulf Hansson [off-list ref] wrote:
On 6 June 2017 at 18:24, John Stultz [off-list ref] wrote:
quoted
On Tue, Jun 6, 2017 at 7:13 AM, Ulf Hansson [off-list ref] wrote:
quoted
On 6 June 2017 at 12:08, Ulf Hansson [off-list ref] wrote:
quoted
[...]
quoted
quoted
quoted
John, thanks for the report!
After a some investigation, I realized that the mmc pwrseq_simple
driver returns -EPROBE_DEFER as it's not able to get the "ext" clock.
Hence the SDIO card will not be detected.
To fix the problem, you need to enable CONFIG_COMMON_CLK_HI655X in the
kernel config. This is actually also the case when using the arm64
defconfig for a plain 4.12 rc3 and later. We should probably make this
driver enabled per default when building the arm64 defconfig.
Can you run a re-test at your side with the CONFIG_COMMON_CLK_HI655X
set? Just to make sure it works also with those boot binaries you are
using...
So unfortunately, now I'm seeing a side-effect from enabling
CONFIG_COMMON_CLK_HI655X.
Using the serial-dev driver for the TI bluetooth, it seems I'm seeing
failures when CONFIG_COMMON_CLK_HI655X is enabled, but configuring it
out, bluetooth works (and wifi then fails).
Looking through the dmesg logs, the bluetooth driver seems to
initialize up properly and load the firmware, but with CLK_HI655X
enabled, we see:
Bluetooth: hci0 command 0xff05 tx timeout
Bluetooth: hci0: send command failed\x0a
Bluetooth: hci0 command 0xff36 tx timeout
Bluetooth: hci0 command 0x1003 tx timeout
Bluetooth: hci0 command 0x1001 tx timeout
Bluetooth: hci0 command 0x1009 tx timeout
...
As Rob said below, this is because the blue-tooth driver doesn't
properly deal with the power on/off sequence.
I guess it works for you because your versions of the boot binaries
turns all needed resources on. That isn't the case for me, blue-tooth
is neither working before or after this change, but wifi is.
quoted
quoted
This effectively seems to make bluetooth an wifi functionality
exclusive. So I don't think this is a sufficient solution (over
reverting the original patch that changes the dts - which allows both
to function).
Since the issues you are reporting about is depending on the
boot-binaries and not a really bugs in the kernel, may I suggest that
we invest our efforts in fixing the blue-tooth driver instead, as it's
there the real problem is!?
quoted
If the clock to the TI chip is described in DT for WiFi, then the BT
side needs it as well (as does the driver) for proper refcounting. The
same would apply to regulators as well. I looked at the regulators for
HiKey and the 2 supplies (Vbat and i/o) appear to both be always on.
Rob
Correct.
I am working on patch, I keep you on cc.
Rob, John,
I have now looked into this a bit more. So I have some local patches,
which in principle adds the external clock to the bluetooth node for
the hikey dts, and adapts the uart bluetooth driver
(drivers/bluetooth/hci_ll.c) to also take the ext clock into
consideration while powering on/off the chip. I am working on a
vanilla kernel, thus not John's tree.
However, I can't get the uart1 amba device to be added because of this
error during boot:
"OF: amba_device_add() failed (-19) for /soc/uart at f7111000"
The reason why it fails is because amba_device_try_add() fails to read
the amba periphid of the uart1 device. I haven't yet been able to
figure out why.
Is this problem something you have seen before? I tried out v4.11,
v4.12-rc3 and John's tree (dev/hikey-mainline-WIP which is based upon
4.12.rc3), they all suffer from the same problem, not being able to
add the uart1 device.
The consequence then becomes that the bluetooth node (which is a child
node for the uart1 node), added in the below commit by Rob, never gets
parsed and thus the device don't become added. In other words, I
haven't been able to test my changes since I can't even get the
bluetooth device to be added.
John, are you using the pcm bluetooth interface or the uart?
UART.
I'm not sure why initializing the UART fails for you. I suspect again
it might be related to differences in the bootloader, as I'm not sure
if uboot has had nearly the amount of usage as UEFI.
You are probably right, but that also makes me worried, as we have
likely other similar issues where UEFI just magically solves things
for the kernel.
Yea. If I recall there were a few other clks (like with the gpu) where
the initialization was moved to UEFI.
Seems like I need to give UEFI a try again, however this time I would
really appreciate your help in testing as I am running out of
bandwidth for this task.
Just about to post the patches....
Sure. I'm happy to test and help tinker.
Sorry for my nit-picking of this issue is causing a burden. But
chasing down these sort of things in every release is a load on my
side too. :)
thanks
-john
From: Daniel Lezcano <hidden> Date: 2017-06-09 15:46:58
On Tue, Jun 06, 2017 at 04:17:40PM +0200, Ulf Hansson wrote:
On 5 June 2017 at 20:13, Daniel Lezcano [off-list ref] wrote:
quoted
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
Acked-by: Ulf Hansson <redacted>
Would be nice to get this trivial fix in for 4.12 rcs to make the WiFi
work for Hikey.
I'm wondering if I submitted this patch for the right path.
Shall it go through arm-soc ?
+Olof, +Arnd
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
On Tue, Jun 06, 2017 at 04:17:40PM +0200, Ulf Hansson wrote:
quoted
On 5 June 2017 at 20:13, Daniel Lezcano [off-list ref] wrote:
quoted
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
Acked-by: Ulf Hansson <redacted>
Would be nice to get this trivial fix in for 4.12 rcs to make the WiFi
work for Hikey.
I'm wondering if I submitted this patch for the right path.
Shall it go through arm-soc ?
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Arnd
From: John Stultz <hidden> Date: 2017-06-09 20:15:21
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Tue, Jun 06, 2017 at 04:17:40PM +0200, Ulf Hansson wrote:
quoted
On 5 June 2017 at 20:13, Daniel Lezcano [off-list ref] wrote:
quoted
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
Acked-by: Ulf Hansson <redacted>
Would be nice to get this trivial fix in for 4.12 rcs to make the WiFi
work for Hikey.
I'm wondering if I submitted this patch for the right path.
Shall it go through arm-soc ?
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
thanks
-john
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Tue, Jun 06, 2017 at 04:17:40PM +0200, Ulf Hansson wrote:
quoted
On 5 June 2017 at 20:13, Daniel Lezcano [off-list ref] wrote:
quoted
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
Acked-by: Ulf Hansson <redacted>
Would be nice to get this trivial fix in for 4.12 rcs to make the WiFi
work for Hikey.
I'm wondering if I submitted this patch for the right path.
Shall it go through arm-soc ?
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Arnd
From: Daniel Lezcano <hidden> Date: 2017-06-12 09:39:01
On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote:
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Tue, Jun 06, 2017 at 04:17:40PM +0200, Ulf Hansson wrote:
quoted
On 5 June 2017 at 20:13, Daniel Lezcano [off-list ref] wrote:
quoted
With the addition of the hi655x common clock, the config option is missing
for the ARM64's hi6220 platform. That leads to a non functionnal WiFi because
the hi655x clock driver misses when initializing the power sequence via DT.
Cc: John Stultz <redacted>
Cc: Ulf Hansson <redacted>
Cc: Wei Xu <xuwei5@hisilicon.com>
Signed-off-by: Daniel Lezcano <redacted>
Acked-by: Ulf Hansson <redacted>
Would be nice to get this trivial fix in for 4.12 rcs to make the WiFi
work for Hikey.
I'm wondering if I submitted this patch for the right path.
Shall it go through arm-soc ?
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Actually, I share John's opinion.
Ideally when we choose a platform, all the relevants devices configuration
options should be selected automatically from a single topmost node of a tree
(platform selection) to all the nodes corresponding to the devices, leaving the
user to select one simple option without knowledge of the SoC hardware
internals.
If the user is expert in the platform and knows exactly what he does, then he
can select an _EXPERT_ like option and be able to disable some drivers.
It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC'
is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when
MFD_HI655X_PMIC is enabled?
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Mon, Jun 12, 2017 at 11:38 AM, Daniel Lezcano
[off-list ref] wrote:
On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote:
quoted
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Actually, I share John's opinion.
Ideally when we choose a platform, all the relevants devices configuration
options should be selected automatically from a single topmost node of a tree
(platform selection) to all the nodes corresponding to the devices, leaving the
user to select one simple option without knowledge of the SoC hardware
internals.
If the user is expert in the platform and knows exactly what he does, then he
can select an _EXPERT_ like option and be able to disable some drivers.
It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC'
is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when
MFD_HI655X_PMIC is enabled?
I don't think it's that easy. When you do that, MFD_HI655X_PMIC gains
a dependency on COMMON_CLK and will again cause a warning on
machines that disable that during compile testing.
Using 'select' for user-selectable options generally leads to problems,
and you are better off avoiding it. If you want to make the symbol impossible
to turn off for non-EXPERT configurations, you can write it like
config COMMON_CLK_HI655X
tristate "Clock driver for Hi655x" if EXPERT
depends on (MFD_HI655X_PMIC || COMPILE_TEST)
depends on REGMAP
default MFD_HI655X_PMIC
That way the option is completely hidden for non-EXPERT,
but still has the right default otherwise, and the dependencies
are tracked right for compile-testing.
Arnd
From: Daniel Lezcano <hidden> Date: 2017-06-13 12:49:02
On 12/06/2017 23:12, Arnd Bergmann wrote:
On Mon, Jun 12, 2017 at 11:38 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote:
quoted
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Actually, I share John's opinion.
Ideally when we choose a platform, all the relevants devices configuration
options should be selected automatically from a single topmost node of a tree
(platform selection) to all the nodes corresponding to the devices, leaving the
user to select one simple option without knowledge of the SoC hardware
internals.
If the user is expert in the platform and knows exactly what he does, then he
can select an _EXPERT_ like option and be able to disable some drivers.
It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC'
is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when
MFD_HI655X_PMIC is enabled?
I don't think it's that easy. When you do that, MFD_HI655X_PMIC gains
a dependency on COMMON_CLK and will again cause a warning on
machines that disable that during compile testing.
This issue is related to the missing stubs in the includes.
Using 'select' for user-selectable options generally leads to problems,
and you are better off avoiding it. If you want to make the symbol impossible
to turn off for non-EXPERT configurations, you can write it like
config COMMON_CLK_HI655X
tristate "Clock driver for Hi655x" if EXPERT
depends on (MFD_HI655X_PMIC || COMPILE_TEST)
depends on REGMAP
default MFD_HI655X_PMIC
That way the option is completely hidden for non-EXPERT,
but still has the right default otherwise, and the dependencies
are tracked right for compile-testing.
From: Daniel Lezcano <hidden> Date: 2018-02-16 17:35:58
On 12/06/2017 23:12, Arnd Bergmann wrote:
On Mon, Jun 12, 2017 at 11:38 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote:
quoted
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Actually, I share John's opinion.
Ideally when we choose a platform, all the relevants devices configuration
options should be selected automatically from a single topmost node of a tree
(platform selection) to all the nodes corresponding to the devices, leaving the
user to select one simple option without knowledge of the SoC hardware
internals.
If the user is expert in the platform and knows exactly what he does, then he
can select an _EXPERT_ like option and be able to disable some drivers.
It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC'
is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when
MFD_HI655X_PMIC is enabled?
I don't think it's that easy. When you do that, MFD_HI655X_PMIC gains
a dependency on COMMON_CLK and will again cause a warning on
machines that disable that during compile testing.
Using 'select' for user-selectable options generally leads to problems,
and you are better off avoiding it. If you want to make the symbol impossible
to turn off for non-EXPERT configurations, you can write it like
config COMMON_CLK_HI655X
tristate "Clock driver for Hi655x" if EXPERT
depends on (MFD_HI655X_PMIC || COMPILE_TEST)
depends on REGMAP
default MFD_HI655X_PMIC
That way the option is completely hidden for non-EXPERT,
but still has the right default otherwise, and the dependencies
are tracked right for compile-testing.
What about the options:
CONFIG_HI3660_MBOX
CONFIG_HI6220_MBOX
CONFIG_STUB_CLK_HI6220
CONFIG_STUB_CLK_HI3660
?
Would make sense to do something like:
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 16 February 2018 at 19:35, Daniel Lezcano [off-list ref] wrote:
On 12/06/2017 23:12, Arnd Bergmann wrote:
quoted
On Mon, Jun 12, 2017 at 11:38 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote:
quoted
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Actually, I share John's opinion.
Ideally when we choose a platform, all the relevants devices configuration
options should be selected automatically from a single topmost node of a tree
(platform selection) to all the nodes corresponding to the devices, leaving the
user to select one simple option without knowledge of the SoC hardware
internals.
If the user is expert in the platform and knows exactly what he does, then he
can select an _EXPERT_ like option and be able to disable some drivers.
It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC'
is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when
MFD_HI655X_PMIC is enabled?
I don't think it's that easy. When you do that, MFD_HI655X_PMIC gains
a dependency on COMMON_CLK and will again cause a warning on
machines that disable that during compile testing.
Using 'select' for user-selectable options generally leads to problems,
and you are better off avoiding it. If you want to make the symbol impossible
to turn off for non-EXPERT configurations, you can write it like
config COMMON_CLK_HI655X
tristate "Clock driver for Hi655x" if EXPERT
depends on (MFD_HI655X_PMIC || COMPILE_TEST)
depends on REGMAP
default MFD_HI655X_PMIC
That way the option is completely hidden for non-EXPERT,
but still has the right default otherwise, and the dependencies
are tracked right for compile-testing.
What about the options:
First, as distros, automatic selection down from selecting ARCH_X is
preferred over
defconfigs. However, we also prefer to build everything possible as
modules, so "default Y"
is sometimes too strong.
CONFIG_HI3660_MBOX
CONFIG_HI6220_MBOX
These are tristate and platorms can boot without them.
From: Daniel Lezcano <hidden> Date: 2018-02-21 10:34:36
On 21/02/2018 11:30, Riku Voipio wrote:
On 16 February 2018 at 19:35, Daniel Lezcano [off-list ref] wrote:
quoted
On 12/06/2017 23:12, Arnd Bergmann wrote:
quoted
On Mon, Jun 12, 2017 at 11:38 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
On Fri, Jun 09, 2017 at 10:48:13PM +0200, Arnd Bergmann wrote:
quoted
On Fri, Jun 9, 2017 at 10:15 PM, John Stultz [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 1:06 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Jun 9, 2017 at 5:46 PM, Daniel Lezcano
[off-list ref] wrote:
Yes, but I'm not sure this is the right patch either. We tend to not
use 'select' for user-visible drivers, and most hisilicon platforms
won't need this driver.
I think it would be more consistent to add this to the defconfig
and regard it as a user error when the driver is disabled on a
machine that needs it.
Maybe the select is not exactly in the right place, but I don't really
feel like a pmic on an SoC is a "user-visible driver". I deal with the
board often and when the new dependency was made on the clk, I would
have never have found it on my own w/o Ulf and Daniel pointing out
what I needed to enable.
What I meant is that the Kconfig option is user-visible. On a very high
level, this is a result of arch/arm64/Kconfig.platforms listing only
very broad categories of SoCs, in many cases only the manufacturers
of very different chip families, which then control the visibility of the
individual Kconfig items for things like pinctrl or clk.
I now see that MFD_HI655X_PMIC is the top-level driver that you
have to select before enabling COMMON_CLK_HI655X, so the
patch is actually broken unless it actually selects both.
How about simply adding a 'default MFD_HI655X_PMIC' to
COMMON_CLK_HI655X to enable it unless it is explicitly
turned off?
Actually, I share John's opinion.
Ideally when we choose a platform, all the relevants devices configuration
options should be selected automatically from a single topmost node of a tree
(platform selection) to all the nodes corresponding to the devices, leaving the
user to select one simple option without knowledge of the SoC hardware
internals.
If the user is expert in the platform and knows exactly what he does, then he
can select an _EXPERT_ like option and be able to disable some drivers.
It is how I tend to write the Kconfig options, so the 'default MFD_HI655X_PMIC'
is confusing for me. Wouldn't make sense to select COMMON_CLK_HI655X when
MFD_HI655X_PMIC is enabled?
I don't think it's that easy. When you do that, MFD_HI655X_PMIC gains
a dependency on COMMON_CLK and will again cause a warning on
machines that disable that during compile testing.
Using 'select' for user-selectable options generally leads to problems,
and you are better off avoiding it. If you want to make the symbol impossible
to turn off for non-EXPERT configurations, you can write it like
config COMMON_CLK_HI655X
tristate "Clock driver for Hi655x" if EXPERT
depends on (MFD_HI655X_PMIC || COMPILE_TEST)
depends on REGMAP
default MFD_HI655X_PMIC
That way the option is completely hidden for non-EXPERT,
but still has the right default otherwise, and the dependencies
are tracked right for compile-testing.
What about the options:
First, as distros, automatic selection down from selecting ARCH_X is
preferred over
defconfigs. However, we also prefer to build everything possible as
modules, so "default Y"
is sometimes too strong.
quoted
CONFIG_HI3660_MBOX
CONFIG_HI6220_MBOX
These are tristate and platorms can boot without them.