Page MenuHomeFreeBSD
Differential D43034

jh7110_gpio: Add StarFive JH7110 GPIO controller driver
Needs ReviewPublic
Actions

Authored by jsihv_gmx.com on Dec 13 2023, 4:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 18, 8:26 AM
Unknown Object (File)
Fri, Mar 14, 2:55 PM
Unknown Object (File)
Feb 17 2025, 2:38 AM
Unknown Object (File)
Feb 16 2025, 5:23 AM
Unknown Object (File)
Feb 14 2025, 7:39 PM
Unknown Object (File)
Jan 28 2025, 7:02 AM
Unknown Object (File)
Jan 28 2025, 6:55 AM
Unknown Object (File)
Jan 14 2025, 5:04 AM
View All 71 Files
Subscribers
emaste
imp
loos

Details

Reviewers
None
Group Reviewers
riscv

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

jsihv_gmx.com created this revision.Dec 13 2023, 4:14 PM
Herald added subscribers: loos, imp. · View Herald TranscriptDec 13 2023, 4:14 PM
jsihv_gmx.com requested review of this revision.Dec 13 2023, 4:14 PM
Harbormaster completed remote builds in B54917: Diff 131330.Dec 13 2023, 4:14 PM
jrtc27 added a reviewer: riscv.Dec 14 2023, 6:50 PM
jrtc27 retitled this revision from adding jh7110 gpio to jh7110_gpio: Add StarFive JH7110 GPIO controller driver.Dec 14 2023, 7:11 PM
jrtc27 edited the summary of this revision. (Show Details)
jsihv_gmx.com updated this revision to Diff 131454.Dec 14 2023, 9:30 PM
  • fixing style issues
Harbormaster completed remote builds in B54957: Diff 131454.Dec 14 2023, 9:30 PM
jsihv_gmx.com updated this revision to Diff 131455.Dec 14 2023, 9:58 PM

I blew up the previous fix. Now fixing the (style) fix.

jsihv_gmx.com planned changes to this revision.Jan 31 2024, 7:33 PM

This code will be adjusted to support a device tree supplied with U-Boot

jsihv_gmx.com updated this revision to Diff 134840.Feb 22 2024, 2:53 PM

Switching to another device tree. Makes no really difference for this file.

emaste added a subscriber: emaste.Jun 18 2024, 3:34 PM
jsihv_gmx.com updated this revision to Diff 147598.Dec 6 2024, 4:23 PM

Updating clk.h's path which has been changed during this year and adding this file to files.starfive

Revision Contents
Changeset List

PathSize
sys/
riscv/
starfive/
315 lines

Diff 131455

View Options

sys/riscv/starfive/jh7110_gpio.c

Context not available.
static void tcp_usrclosed(struct tcpcb *); static void tcp_usrclosed(struct tcpcb *);
static void tcp_fill_info(struct tcpcb *, struct tcp_info *); static void tcp_fill_info(struct tcpcb *, struct tcp_info *);
static int tcp_options_support(struct tcpcb *tp, int flags);
#ifdef TCPDEBUG #ifdef TCPDEBUG
#define TCPDEBUG0 int ostate = 0 #define TCPDEBUG0 int ostate = 0
#define TCPDEBUG1() ostate = tp ? tp->t_state : 0 #define TCPDEBUG1() ostate = tp ? tp->t_state : 0
Context not available.
goto out; goto out;
} }
tp = intotcpcb(inp); tp = intotcpcb(inp);
if (flags & PRUS_OOB) {
rscheffUnsubmitted
Not Done
Inline Actions

the socket MSG_OOB flag is directly mapped into protocol PRU_OOB flag, not? (they also happen to be numeric identical). Conceptually, the protocol itself should be checking the PRUS_* flags, not the MSG_* flags.

uipc_socket.c:1499: (flags & MSG_OOB) ? PRUS_OOB :
uipc_socket.c:1729: pru_flag = (flags & MSG_OOB) ? PRUS_OOB :

rscheff: the socket MSG_OOB flag is directly mapped into protocol PRU_OOB flag, not? (they also happen…
rrsAuthorUnsubmitted
Done
Inline Actions

The problem here can be found in uipc_socket.c. Before this function became the pru_send routine we "used" to
use sosend_generric() or maybe sosend_stream(), go look at these two functions (in fact look too at sosend_dgram)
*all* of them map

MSG_OOB -> PRUS_OOB
and
MSG_EOF -> PRUS_EOF

But now, since this function *is* what is called that is not happening, since this function does nto
have the proper mapping.

Yes conceptually the protocol needs to use PRUS_... but someone must do the translation from
the socket level to the protocol level. And that *used* to be done in sosend_xxxx() function which now
is no longer called. It most likely should be done here.

rrs: The problem here can be found in uipc_socket.c. Before this function became the pru_send…
rscheffUnsubmitted
Not Done
Inline Actions

Then I must be missing some context here.

Are you saying that you are bypassing the socket interface, and instead are calling tcp_usr_send() directly in some deployments, where the MSG_ flags are used instead of the PRUS_ flags?

kern_sendfile.c has vn_sendfile, this however doesn't support OOB either apparently.

rscheff: Then I must be missing some context here. Are you saying that you are bypassing the socket…
rscheffUnsubmitted
Not Done
Inline Actions

see transport@; I believe you missed that sosend() is calling the pru_sosend() pointer, and not the pru_send() pointer. So the socket API for TCP still runs thought either sosend_generic (default) or sosend_stream (sysctl selectable), both of which do the conversion from MSG_OOB / MSG_EOF in the "correct" layer - and TCP here only has to care about the PRU_ type flags.

rscheff: see transport@; I believe you missed that sosend() is calling the pru_sosend() pointer, and not…
rrsAuthorUnsubmitted
Done
Inline Actions

Yeah I had missed the so

rrs: Yeah I had missed the so
if ((error = tcp_options_support(tp, PRUS_OOB)) != 0) {
if (control)
m_freem(control);
if (m && (flags & PRUS_NOTREADY) == 0)
m_freem(m);
error = ECONNRESET;
goto out;
}
}
tuexenUnsubmitted
Not Done
Inline Actions

I think letting tcp_options_support() return the error and lateron overwriting it is not optimal. What about letting tcp_options_support() return a bool, and replace the above with:

if (flags & PRUS_OOB && !tcp_options_support(tp, PRUS_OOB)) {
	if (control)
		m_freem(control);
	if (m && (flags & PRUS_NOTREADY) == 0)
		m_freem(m);
	error = EINVAL;
	goto out;
}
tuexen: I think letting `tcp_options_support()` return the error and lateron overwriting it is not…
rrsAuthorUnsubmitted
Done
Inline Actions

That was a code cloning error...

The error = CONNRESET

should not be present.

and I prefer not to make it a bool, but to have the stack return
0 = I support it
ERRNO = I don't support it and here is the error.

The overwrite was just a mistake.

rrs: That was a code cloning error... The error = CONNRESET should not be present. and I prefer…
TCPDEBUG1(); TCPDEBUG1();
if (nam != NULL && tp->t_state < TCPS_SYN_SENT) { if (nam != NULL && tp->t_state < TCPS_SYN_SENT) {
switch (nam->sa_family) { switch (nam->sa_family) {
Context not available.
NET_EPOCH_EXIT(et); NET_EPOCH_EXIT(et);
} }
static int
tcp_options_support(struct tcpcb *tp, int flags)
tuexenUnsubmitted
Not Done
Inline Actions

Maybe let it return a bool? Maybe call it something like tcp_pru_options_support() or so? TCP options rings other bells...

tuexen: Maybe let it return a `bool`? Maybe call it something like `tcp_pru_options_support()` or so?
rrsAuthorUnsubmitted
Done
Inline Actions

sure

rrs: sure
{
/*
* For now we say we support anything, we
* need to change this so that we ask the
* specific stack and let it decide if
* it supports PRUS_OOB or possibly other
* flags in the future.
*/
return (0);
}
/* /*
* Receive out-of-band data. * Receive out-of-band data.
*/ */
Context not available.
goto out; goto out;
} }
tp = intotcpcb(inp); tp = intotcpcb(inp);
error = tcp_options_support(tp, PRUS_OOB);
if (error) {
goto out;
tuexenUnsubmitted
Not Done
Inline Actions

If we use a bool, we want to set error to something....

tuexen: If we use a bool, we want to set error to something....
rrsAuthorUnsubmitted
Done
Inline Actions

I do not want to use bool here

rrs: I do not want to use bool here
}
TCPDEBUG1(); TCPDEBUG1();
if ((so->so_oobmark == 0 && if ((so->so_oobmark == 0 &&
(so->so_rcv.sb_state & SBS_RCVATMARK) == 0) || (so->so_rcv.sb_state & SBS_RCVATMARK) == 0) ||
Context not available.