[Bug 271062] [PATCH] add support for 28xx based device to isp(4)
- In reply to: bugzilla-noreply_a_freebsd.org: "[Bug 271062] [PATCH] add support for 28xx based device to isp(4)"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 25 Apr 2023 16:17:54 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271062
Warner Losh <imp@FreeBSD.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |imp@FreeBSD.org
--- Comment #1 from Warner Losh <imp@FreeBSD.org> ---
I generally like this patch... however...
- isp_prt(isp, ISP_LOGERR, "bad frame length (%d) from NVRAM-
using %d", DEFAULT_FRAMESIZE(isp), ICB_DFLT_FRMLEN);
- icbp->icb_maxfrmlen = ICB_DFLT_FRMLEN;
+ if (IS_28XX(isp)) {
+ isp_prt(isp, ISP_LOGERR, "bad frame length (%d) from
NVRAM - using %d", DEFAULT_FRAMESIZE(isp), ICB_DFLT_FRMLEN_28XX);
+ icbp->icb_maxfrmlen = ICB_DFLT_FRMLEN_28XX;
+ } else {
+ isp_prt(isp, ISP_LOGERR, "bad frame length (%d) from
NVRAM - using %d", DEFAULT_FRAMESIZE(isp), ICB_DFLT_FRMLEN);
+ icbp->icb_maxfrmlen = ICB_DFLT_FRMLEN;
+ }
looks a little ugly to me. I'd be tempted to add a icbp->icb_dflt_frmlen field
where we detect the 2800, set a different value. That way, we'd not need the if
here with the code duplication.
Other than that, the patch looks very good.
--
You are receiving this mail because:
You are the assignee for the bug.