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.

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

Rpmsg gpiodrv #247

Open
wants to merge 259 commits into
base: 4.14
Choose a base branch
from
Open

Rpmsg gpiodrv #247

wants to merge 259 commits into from

Conversation

deebot
Copy link

@deebot deebot commented Aug 23, 2020

This is a driver that enables the use of sysfs interface and chardev interface with libgpiod to send and receive messages to the PRU. Please review it . You might also want to look into my repo where i have used it to control shift register. https://github.com/deebot/Beaglebone-BidirectionBus/tree/dev

RobertCNelson and others added 30 commits August 24, 2020 18:17
Reference: v5.3.18
Signed-off-by: Robert Nelson <[email protected]>
Signed-off-by: Robert Nelson <[email protected]>
Reference: v5.0.21
Signed-off-by: Robert Nelson <[email protected]>
Signed-off-by: Robert Nelson <[email protected]>
Reference: v4.14.77
Signed-off-by: Robert Nelson <[email protected]>
The firmware for brcmfmac devices includes information regarding
regulatory constraints. For certain devices this information is kept
separately in a binary form that needs to be downloaded to the device.
This patch adds support to download this so-called CLM blob file. It
uses the same naming scheme as the other firmware files with extension
of .clm_blob.

The CLM blob file is optional. If the file does not exist, the download
process will be bypassed. It will not affect the driver loading.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Chung-Hsien Hsu <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
We got SDIO_CRC_ERROR with 4373 on SDR104 when doing bi-directional
throughput test. Setting F2 blocksize and enable watermark to 256 to
guarantee the operation stability.

Signed-off-by: Wright Feng <[email protected]>
broken_sg_support, sd_head_align, and sd_sgentry_align are used in
brcmfmac code but not configurable in dts file. Add the parsing logic.
Now they can be configured like below in dts:
	brcm,broken_sg_support;
	brcm,sd_head_align = /bits/ 16 <4>;
	brcm,sd_sgentry_align = /bits/ 16 <4>;

Signed-off-by: Chi-hsien Lin <[email protected]>
Firmware returns proprietary error code when getting error in
fil_cmd_data_set or fil_cmd_data_get. Sometimes the vendor tool or
utilities which uses libnl may stuck in some commands when wl is down.
For example, issue "scan" command after issuing "down" command, the
"scan" command will be the blocking call and stuck as no response from
firmware. It is caused by that firmware returns BCME_NOTUP(-4) when wl
is down, but in Linux the -4 is -EINTR, so libnl catches the error and
not pass to upper layer.
Because of that, the driver should return Linux error code instead of the
proprietary error code, and the tools or utilities need to get the real
firmware error code by another command "bcmerrorstr" after receiving
the error.

Signed-off-by: Wright Feng <[email protected]>
CYW43012 is a 1x1 802.11a/b/g/n Dual-Band HT20, 256-QAM/Turbo QAM. It
is an Ultra Low Power WLAN+BT combo chip.

Signed-off-by: Chi-Hsien Lin <[email protected]>
APSTA can work on two band concurrently with using VSDB(Virtual
Simultaneous Dual-Band) or RSDB(Real Simultaneous Dual-Band) features.
In this case, we have to keep apsta is 1 in firmware side.
If we start wpa_supplicant on wlan0 and then start hostapd on wlan1, the
apsta will be set to 0, and data will be stall on wlan0(station).
Because that, we only set apsta to 0 when AP start on primary interface.

Signed-off-by: Wright Feng <[email protected]>
Saverestore register settings for 43012.

Signed-off-by: Praveen Babu Chandran <[email protected]>
This patch will add 43455 into the save-restore(SR) capable chip list, so
the SR engine will be enabled with 43455 FW which built-in the -sr
function.

Signed-off-by: Lo-Hsiang Lo <[email protected]>
…bled

For legacy chips w/o CLM blob files, kernel with user helper function
enabled returns -EAGAIN when we request_firmware() for blob file:
"request_firmware() -> _request_firmware() -> fw_load_from_user_helper()
-> _request_firmware_load() -> retval=-EAGAIN"
We should do retries and also continue brcmf_c_process_clm_blob if
getting -EAGAIN from request_firmware function.

Signed-off-by: Wright Feng <[email protected]>
The buffer size of return of cap iovar is greater than 256 bytes in
some firmares. For instance, the return size of cap iovar is 271 bytes
in 4373 13.10.246.79 firmare. It makes caps buffer is default value and
cause the feature capability parsing failed.
Because of that, we enlarge caps buffer size to 512 bytes and add
the error print for cap iovar error.

Signed-off-by: Wright Feng <[email protected]>
Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx
packet to reduce latency. In fcmode 1/2, host driver enqueues skb in
hanger and TCP doesn't push new skb frees until host frees the skb when
receiving fwstatus event. So using skb_orphan before sending skb to bus
will make the skb removing the ownership of socket. With this patch, we
got better throughput in fcmode 1/2.

Tested 43455 TCP throughput in 20 MHz bandwidth with/without this patch.
fcmode 0: 59.5 / 59.6 (Mbps)
fcmode 1: 59.3 / 23.4 (Mbps)
fcmode 2: 59.6 / 21.5 (Mbps)

Signed-off-by: Wright Feng <[email protected]>
…DP RX Traffic.

The number of words that the read FIFO has to contain except
the end of frame before sends data back to the host.
Max watermark = (512B - 2* (BurstLength))/4 =
(512 - 128)/4 = 384/4 = 0x60
so if burst length (i.e. BurstLength = 64) is increased,
watermark has to be reduced. This is the optimal setting for this chip.

Signed-off-by: Naveen Gupta <[email protected]>
In Deep Sleep mode ARM is off and once Exit trigger comes than
Mail Box Interrupt comes to Host and whole Re Initiation should be done
in the ARM to start TX/RX.

Signed-off-by: Naveen Gupta <[email protected]>
Add WLAN_AKM_SUITE_FT_8021X and WLAN_AKM_SUITE_FT_PSK in
brcmf_set_key_mgmt() for FT support.

Signed-off-by: Chung-Hsien Hsu <[email protected]>
Signed-off-by: Chi-hsien Lin <[email protected]>
Hostap daemon has a parameter "ap_isolate which is used to prevent
low-level bridging of frames between associated stations in the BSS.
For driver side, we add cfg80211 ops method change_bss to support
setting AP isolation from user space.

Signed-off-by: Wright Feng <[email protected]>
Signed-off-by: Chi-hsien Lin <[email protected]>
Don't print ulp_sdioctrl get error as errors are expected for non ulp cases.

Signed-off-by: Naveen Gupta <[email protected]>
There is a system warning message, warn_slowpath-fmt, during suspend
while using supplicant join AP and enable wowl feature by IW command.
It's cuased by brcmf_pno_remove_request path can't find the reqid.
This fix will not go to remove pno request function if there is no
pno scan.

Signed-off-by: Lo-Hsiang Lo <[email protected]>
To enhance RX throughput, we add a module parameter "sdio_dpc_prio" to let
user can set scheduling  priority for sdio_dpc. It can improve RX
throughput by reducing the receiving time in sdio_dpc.

Signed-off-by: Wright Feng <[email protected]>
When eap_restrict is enabled, firmware will toss non-802.1x frames from
tx/rx data path if station not yet authorized.
Internal firmware eap_restrict is disabled by default. This patch makes
it possible to enable firmware eap_restrict by specifying
eap_restrict=1 as module parameter.

Signed-off-by: Wright Feng <[email protected]>
FMAC driver need to provide a dummy wowlan filter for kernel and
provided the well configured wowlan stack. So the system will
keep driver in connected state in suspend mode and can be wake
up by ping packet.

Enable unicast packet filter before system suspend and
disable it after resume.

Signed-off-by: Lo-Hsiang Lo <[email protected]>
Set wowl configuration in disconnect state is redundant.
Remove it to fix no scan result issue after resume.

Signed-off-by: Lo-Hsiang Lo <[email protected]>
RobertCNelson and others added 12 commits August 24, 2020 18:18
This crashes the kernel when an runtime overlay is applied.
Signed-off-by: Robert Nelson <[email protected]>
We have errata i688 workaround produce warnings on SoCs other than
omap4 and omap5:

omap4_sram_init:Unable to allocate sram needed to handle errata I688
omap4_sram_init:Unable to get sram pool needed to handle errata I688

This is happening because there is no ti,omap4-mpu node, or no SRAM
to configure for the other SoCs, so let's remove the warning based
on the SoC revision checks.

As nobody has complained it seems that the other SoC variants do not
need this workaround.

Signed-off-by: Tony Lindgren <[email protected]>
already default on BBB/X15, just calcuated everybootup

Signed-off-by: Robert Nelson <[email protected]>
commit e33a814 upstream.

gcc 10 will default to -fno-common, which causes this error at link
time:

  (.text+0x0): multiple definition of `yylloc'; dtc-lexer.lex.o (symbol from plugin):(.text+0x0): first defined here

This is because both dtc-lexer as well as dtc-parser define the same
global symbol yyloc. Before with -fcommon those were merged into one
defintion. The proper solution would be to to mark this as "extern",
however that leads to:

  dtc-lexer.l:26:16: error: redundant redeclaration of 'yylloc' [-Werror=redundant-decls]
   26 | extern YYLTYPE yylloc;
      |                ^~~~~~
In file included from dtc-lexer.l:24:
dtc-parser.tab.h:127:16: note: previous declaration of 'yylloc' was here
  127 | extern YYLTYPE yylloc;
      |                ^~~~~~
cc1: all warnings being treated as errors

which means the declaration is completely redundant and can just be
dropped.

Signed-off-by: Dirk Mueller <[email protected]>
Signed-off-by: David Gibson <[email protected]>
[robh: cherry-pick from upstream]
Cc: [email protected]
Signed-off-by: Rob Herring <[email protected]>
[nc: Also apply to dtc-lexer.lex.c_shipped due to a lack of
     e039139, where dtc-lexer.l started being used]
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Reference: v5.2.21
Signed-off-by: Robert Nelson <[email protected]>
Copy link
Member

@jadonk jadonk left a comment

Choose a reason for hiding this comment

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

I think @RobertCNelson is working on applying GSoC patches to newer kernels, so you might need to rebase against 4.19 or 5.4.

Please combine into a single patch, run checkpatch.pl, add some comments to help it be a good example and provide a more thorough commit message for posterity. :-)

Can you work out having the driver automatically loading the firmware?

@@ -81,4 +81,15 @@ config RPMSG_PRU

If unsure, say N.

config CONFIG_RPMSG_GPIOHIP_INTERFACE
tristate "A rpmsg driver with gpiochip interface"
Copy link
Member

Choose a reason for hiding this comment

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

Watch your whitespace and run checkpatch.pl.

Copy link
Author

@deebot deebot Aug 26, 2020

Choose a reason for hiding this comment

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

Removed white spaces

I think @RobertCNelson is working on applying GSoC patches to newer kernels, so you might need to rebase against 4.19 or 5.4.

Please combine into a single patch, run checkpatch.pl, add some comments to help it be a good example and provide a more thorough commit message for posterity. :-)

Can you work out having the driver automatically loading the firmware?

I will try to implement automatic loading of firmware, also trying to rebase. For now i have amended the previous commit

@@ -9,4 +9,6 @@ obj-$(CONFIG_RPMSG_QCOM_SMD) += qcom_smd.o
obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o

obj-$(CONFIG_RPMSG_RPC) += rpmsg-rpc.o
obj-$(CONFIG_RPMSG_GPIOCHIP_INTERFACE) += rpmsg_gpio.o
Copy link
Member

Choose a reason for hiding this comment

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

Please combine this patch with the source code patch. I don't see any reason for them to be separate.

Copy link
Author

@deebot deebot Aug 26, 2020

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,190 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Please add a description in the commit log. Please include information on the firmware required for the driver to function and steps for testing it.

* PRU Remote Processor Messaging Driver with gpiochip interface
* For example codes to test and run the complete communication between PRU and ARM
* https://github.com/deebot/Beaglebone-BidirectionBus/tree/dev
* This software is licensed under the terms of the GNU General Public
Copy link
Member

Choose a reason for hiding this comment

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

Give some space between documentation and license information for readability. Check out SPDX.

Copy link
Author

Choose a reason for hiding this comment

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

Space added

wait_queue_head_t wait_list;
uint32_t gpio_state;
long input_state ;

Copy link
Member

Choose a reason for hiding this comment

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

Watch whitespace and run checkpatch.pl.

Copy link
Author

Choose a reason for hiding this comment

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

Removed all white spaces errors using checkpatch

return -ENOSPC;
}

length = kfifo_in(&prudev->msg_fifo, data, len);
Copy link
Member

Choose a reason for hiding this comment

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

Is this where the "magic" happens? A comment here might be helpful. I believe this is the whole "meat" of the driver where you simply pass a message to the PRU using rpmsg. For this to be a good example, telling people what is going on here is critical.

Copy link
Author

Choose a reason for hiding this comment

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

This is one standard operation that happens in all rpmsg drivers i have seen. I have added a comment

if (!prudev)
return -ENOMEM;
prudev->rpdev = rpdev;
ret = kfifo_alloc(&prudev->msg_fifo, MAX_FIFO_MSG * FIFO_MSG_SIZE,
Copy link
Member

Choose a reason for hiding this comment

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

Why is the return value ignored? What is this function doing? I imagine it is allocating RPMSG communication buffers, but for this to be an example, it should be well commented. That doesn't mean adding lots and lots of comments, but rather comments in critical locations to put things in the right light. It needs to be clear what is driver boilerplate and what are the custom parts that make this driver interesting.

Copy link
Author

Choose a reason for hiding this comment

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

This was part of boiler plate a standard process in all rpmsg examples i saw. I have added code to catch failour

#define RPMSG_BUF_SIZE (512)
#define MAX_FIFO_MSG (32)
#define FIFO_MSG_SIZE RPMSG_BUF_SIZE
#define GPIO_NUM 9
Copy link
Member

Choose a reason for hiding this comment

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

Why the magic number?

Copy link
Author

Choose a reason for hiding this comment

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

Buffer size and fifo sizes numbers come from virtio_rpmsg.h. the GPIO_NUM is 9 because i am sending 9 bits as payload.
I need 9 lines to send 9 bits.

int msg_idx_rd;
int msg_idx_wr;
wait_queue_head_t wait_list;
uint32_t gpio_state;
Copy link
Member

Choose a reason for hiding this comment

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

What is the structure of this state variable?

Copy link
Author

Choose a reason for hiding this comment

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

This variable has 9 bits of data from 9 lines. If the MSB bit is 1 the PRU gets to know it need to configure the shift register as SIPO and push out the remaing 8 bit data to the LEDs or whatever connected to the shift registor. When this bit is low PRU discards the other 8 bits and configures the shift registor to read the data from I/O and send it back to the linux side.

if(offset==GPIO_NUM-1){
memcpy((void*)&rpmsg_pru_buf,(void*)&(prudev->gpio_state),sizeof(&(prudev->gpio_state)));
pr_info("A check for checking gpio_state: %d",prudev->gpio_state);
ret=rpmsg_send(rpdev->ept, (void *)rpmsg_pru_buf, 2);
Copy link
Member

Choose a reason for hiding this comment

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

Ah, this is where the "magic" happens and the message is passed to the PRU to update the GPIO state. For this to be a good example for those trying to learn about the PRU and RPMSG, it is important to document the critical sections.

Copy link
Author

Choose a reason for hiding this comment

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

Yes here is where the data is packaged in char buf and is sent over to PRU.I have added a comment in the code.

@RobertCNelson
Copy link
Member

I'll still merge this to v4.14.x-ti to give it some testing.

@deebot deebot force-pushed the rpmsg-gpiodrv branch 4 times, most recently from 66cbf64 to 4cc8ca9 Compare August 26, 2020 15:35
…ware

  Please follow instructions to load PRU firmware
  https://github.com/deebot/Beaglebone-BidirectionBus/blob/dev/bidirec_299/README.md
- Added rule to compile rpmsg-gpio driver in Make and Kconfig
RobertCNelson added a commit to RobertCNelson/ti-linux-kernel-dev that referenced this pull request Sep 9, 2020
fixes: beagleboard/linux#247
Signed-off-by: Robert Nelson <[email protected]>
RobertCNelson added a commit to RobertCNelson/ti-linux-kernel-dev that referenced this pull request Sep 9, 2020
fixes: beagleboard/linux#247
Signed-off-by: Robert Nelson <[email protected]>
@RobertCNelson RobertCNelson force-pushed the 4.14 branch 2 times, most recently from cf8aa20 to 6ab314d Compare March 18, 2021 19:07
@RobertCNelson RobertCNelson force-pushed the 4.14 branch 2 times, most recently from bf0870f to 36b21ce Compare May 27, 2021 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.