Page MenuHomeFreeBSD
Differential D10719

Enable proper parsing of nested simlpe-buses on Marvell platforms
ClosedPublic
Actions

Authored by mw_semihalf.com on May 14 2017, 12:55 AM.
Tags
Referenced Files
Unknown Object (File)
Thu, Apr 3, 8:43 AM
Unknown Object (File)
Mar 10 2025, 11:46 PM
Unknown Object (File)
Feb 24 2025, 5:43 PM
Unknown Object (File)
Feb 22 2025, 2:33 PM
Unknown Object (File)
Feb 22 2025, 2:06 PM
Unknown Object (File)
Feb 22 2025, 1:27 PM
Unknown Object (File)
Feb 7 2025, 1:34 PM
Unknown Object (File)
Nov 29 2024, 7:53 AM
View All 73 Files
Subscribers
andrew
fabient
imp

Details

Reviewers
zbb
loos
mmel
• ian
Commits
rS318407: Enable proper parsing of nested simlpe-buses on Marvell platforms
Summary

OF_finddevice doesn't find the "simple-bus" node, which is problematic
for Marvell platforms, using nested buses in Device Tree, like
Armada 38x SoC.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
head/
sys/
arm/
mv/
7 lines

Diff 28467

View Options

head/sys/arm/mv/mv_common.c

Show First 20 Lines • Show All 1,358 Lines • ▼ Show 20 Lines
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
char *data = (char *)ifreqp->ifr_data; char tmpdata;
char *data;
void *info = NULL; void *info = NULL;
int retValue = EINVAL; int retValue = EINVAL;
tmpdata = fubyte(ifreqp->ifr_data);
if (tmpdata == -1)
return (EFAULT);
data = &tmpdata;
switch(*data) { switch(*data) {
jhbUnsubmitted
Not Done
Inline Actions

If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone trying to use additional bytes and instead do something like:

char query;
...

query = fubyte(ifreqp->ifr_data);
if (query == -1)
     return (EFAULT);

switch (query) {

Also, I wonder if we should prefer fuebyte() in new code instead? (Maybe kib@ has an opinion on that)

jhb: If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone…
brooksAuthorUnsubmitted
Not Done
Inline Actions

data is used later or I would have done that. We don't currently have fuebyte()

brooks: `data` is used later or I would have done that. We don't currently have fuebyte()
case XGE_QUERY_STATS: case XGE_QUERY_STATS:
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_stats_hw(lldev->devh, status = xge_hal_stats_hw(lldev->devh,
(xge_hal_stats_hw_info_t **)&info); (xge_hal_stats_hw_info_t **)&info);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(info, ifreqp->ifr_data, if(copyout(info, ifreqp->ifr_data,
sizeof(xge_hal_stats_hw_info_t)) == 0) sizeof(xge_hal_stats_hw_info_t)) == 0)
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Lines if(copyout(&lldev->driver_stats, ifreqp->ifr_data,
sizeof(int)) == 0) sizeof(int)) == 0)
retValue = 0; retValue = 0;
break; break;
case XGE_SET_BUFFER_MODE_1: case XGE_SET_BUFFER_MODE_1:
case XGE_SET_BUFFER_MODE_2: case XGE_SET_BUFFER_MODE_2:
case XGE_SET_BUFFER_MODE_5: case XGE_SET_BUFFER_MODE_5:
*data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N'; *data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N';
if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0)
jhbUnsubmitted
Done
Inline Actions

So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would probably fix this by not abusing data but just using a local 'char mode' and passing '&mode' to copyout().

jhb: So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would…
retValue = 0; retValue = 0;
break; break;
default: default:
xge_trace(XGE_TRACE, "Nothing is matching"); xge_trace(XGE_TRACE, "Nothing is matching");
retValue = ENOTTY; retValue = ENOTTY;
break; break;
} }
return retValue; return retValue;
} }
/** /**
* xge_ioctl_registers * xge_ioctl_registers
* IOCTL to get registers * IOCTL to get registers
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_register_t *data = (xge_register_t *)ifreqp->ifr_data; xge_register_t tmpdata;
xge_register_t *data;
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
int retValue = EINVAL, offset = 0, index = 0; int retValue = EINVAL, offset = 0, index = 0;
int error;
u64 val64 = 0; u64 val64 = 0;
error = copyin(ifreqp->ifr_data, &tmpdata, sizeof(tmpdata));
if (error != 0)
return (error);
data = &tmpdata;
/* Reading a register */ /* Reading a register */
if(strcmp(data->option, "-r") == 0) { if(strcmp(data->option, "-r") == 0) {
data->value = 0x0000; data->value = 0x0000;
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset,
&data->value); &data->value);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0)
jhbUnsubmitted
Not Done
Inline Actions

The fact that it does copyout() already makes this function even more amusing.

jhb: The fact that it does copyout() already makes this function even more amusing.
brooksAuthorUnsubmitted
Not Done
Inline Actions

The strcmp() cases will certainly not trigger in most cases because option is a char[2] followed by an int64_t so there's garbage in the pad unless the did a bzero first.

This driver is best fixed with svn rm, but I'm changing ifr_data use so needed to do a little fixing.

brooks: The strcmp() cases will certainly not trigger in most cases because option is a char[2]…
retValue = 0; retValue = 0;
} }
} }
/* Writing to a register */ /* Writing to a register */
else if(strcmp(data->option, "-w") == 0) { else if(strcmp(data->option, "-w") == 0) {
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset,
data->value); data->value);
▲ Show 20 Lines • Show All 1,982 Lines • Show Last 20 Lines