[PATCH u-boot 0/2] eth/r8152: minor corrections
These are minor corrections for r8152 driver.
Hayes Wang (2): eth/r8152: fix assigning the wrong endpoint eth/r8152: fix typo in register name
drivers/usb/eth/r8152.c | 22 ++++++++++++---------- drivers/usb/eth/r8152.h | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-)
Although I think it never occurs, the code doesn't make sense, because it may allow to assign an IN endpoint to ss->ep_out.
Signed-off-by: Hayes Wang hayeswang@realtek.com --- drivers/usb/eth/r8152.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 61b8683230..7c48663370 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -1354,9 +1354,8 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, struct usb_interface *iface; struct usb_interface_descriptor *iface_desc; int ep_in_found = 0, ep_out_found = 0; - int i; - struct r8152 *tp; + int i;
/* let's examine the device now */ iface = &dev->config.if_desc[ifnum]; @@ -1399,10 +1398,13 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, if ((iface->ep_desc[i].bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { u8 ep_addr = iface->ep_desc[i].bEndpointAddress; - if ((ep_addr & USB_DIR_IN) && !ep_in_found) { - ss->ep_in = ep_addr & - USB_ENDPOINT_NUMBER_MASK; - ep_in_found = 1; + + if (ep_addr & USB_DIR_IN) { + if (!ep_in_found) { + ss->ep_in = ep_addr & + USB_ENDPOINT_NUMBER_MASK; + ep_in_found = 1; + } } else { if (!ep_out_found) { ss->ep_out = ep_addr &
On 5/22/20 10:54 AM, Hayes Wang wrote:
Although I think it never occurs, the code doesn't make sense, because it may allow to assign an IN endpoint to ss->ep_out.
Signed-off-by: Hayes Wang hayeswang@realtek.com
drivers/usb/eth/r8152.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 61b8683230..7c48663370 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -1354,9 +1354,8 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, struct usb_interface *iface; struct usb_interface_descriptor *iface_desc; int ep_in_found = 0, ep_out_found = 0;
- int i;
- struct r8152 *tp;
int i;
/* let's examine the device now */ iface = &dev->config.if_desc[ifnum];
@@ -1399,10 +1398,13 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, if ((iface->ep_desc[i].bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { u8 ep_addr = iface->ep_desc[i].bEndpointAddress;
if ((ep_addr & USB_DIR_IN) && !ep_in_found) {ss->ep_in = ep_addr &USB_ENDPOINT_NUMBER_MASK;ep_in_found = 1;
if (ep_addr & USB_DIR_IN) {if (!ep_in_found) {ss->ep_in = ep_addr &USB_ENDPOINT_NUMBER_MASK;ep_in_found = 1;}
So why don't you rework the code this way instead, to make it easier to understand:
if ((ep_addr & USB_DIR_IN) && !ep_in_found) { ... do in stuff ... }
if ((ep_addr & USB_DIR_OUT) && !ep_out_found) { ... do out stuff ... }
Would that work ?
Marek Vasut [mailto:marex@denx.de]
Sent: Friday, May 22, 2020 9:22 PM
[...]
if ((ep_addr & USB_DIR_IN) && !ep_in_found) {ss->ep_in = ep_addr &USB_ENDPOINT_NUMBER_MASK;ep_in_found = 1;
if (ep_addr & USB_DIR_IN) {if (!ep_in_found) {ss->ep_in = ep_addr &USB_ENDPOINT_NUMBER_MASK;ep_in_found = 1;}So why don't you rework the code this way instead, to make it easier to understand:
Ok, I'll do it and resend this patch. Thanks.
if ((ep_addr & USB_DIR_IN) && !ep_in_found) { ... do in stuff ... }
if ((ep_addr & USB_DIR_OUT) && !ep_out_found) { ... do out stuff ... }
Would that work ?
Yes.
Best Regards, Hayes
The PAL_BDC_CR should be PLA_BDC_CR.
Signed-off-by: Hayes Wang hayeswang@realtek.com --- drivers/usb/eth/r8152.c | 8 ++++---- drivers/usb/eth/r8152.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 7c48663370..f201a1789b 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -711,9 +711,9 @@ static void r8152b_enter_oob(struct r8152 *tp)
rtl_rx_vlan_en(tp, false);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PAL_BDC_CR); + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_BDC_CR); ocp_data |= ALDPS_PROXY_MODE; - ocp_write_word(tp, MCU_TYPE_PLA, PAL_BDC_CR, ocp_data); + ocp_write_word(tp, MCU_TYPE_PLA, PLA_BDC_CR, ocp_data);
ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL); ocp_data |= NOW_IS_OOB | DIS_MCU_CLROOB; @@ -844,9 +844,9 @@ static void r8153_enter_oob(struct r8152 *tp)
rtl_rx_vlan_en(tp, false);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PAL_BDC_CR); + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_BDC_CR); ocp_data |= ALDPS_PROXY_MODE; - ocp_write_word(tp, MCU_TYPE_PLA, PAL_BDC_CR, ocp_data); + ocp_write_word(tp, MCU_TYPE_PLA, PLA_BDC_CR, ocp_data);
ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL); ocp_data |= NOW_IS_OOB | DIS_MCU_CLROOB; diff --git a/drivers/usb/eth/r8152.h b/drivers/usb/eth/r8152.h index 09f1c6178b..c7f62b8b3e 100644 --- a/drivers/usb/eth/r8152.h +++ b/drivers/usb/eth/r8152.h @@ -22,7 +22,7 @@ #define PLA_TEREDO_CFG 0xc0bc #define PLA_MAR 0xcd00 #define PLA_BACKUP 0xd000 -#define PAL_BDC_CR 0xd1a0 +#define PLA_BDC_CR 0xd1a0 #define PLA_TEREDO_TIMER 0xd2cc #define PLA_REALWOW_TIMER 0xd2e8 #define PLA_LEDSEL 0xdd90 @@ -225,7 +225,7 @@ #define TEREDO_RS_EVENT_MASK 0x00fe #define OOB_TEREDO_EN 0x0001
-/* PAL_BDC_CR */ +/* PLA_BDC_CR */ #define ALDPS_PROXY_MODE 0x0001
/* PLA_CONFIG34 */
On 5/22/20 10:54 AM, Hayes Wang wrote:
The PAL_BDC_CR should be PLA_BDC_CR.
Signed-off-by: Hayes Wang hayeswang@realtek.com
drivers/usb/eth/r8152.c | 8 ++++---- drivers/usb/eth/r8152.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
Applied, thanks. Take a look at 1/2 and resend it if applicable.
v2: Reword the patch #1 to make it easier to understand.
v1: These are minor corrections for r8152 driver.
Hayes Wang (2): eth/r8152: fix assigning the wrong endpoint eth/r8152: fix typo in register name
drivers/usb/eth/r8152.c | 22 ++++++++++++---------- drivers/usb/eth/r8152.h | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-)
Although I think it never occurs, the code doesn't make sense, because it may allow to assign an IN endpoint to ss->ep_out.
Signed-off-by: Hayes Wang hayeswang@realtek.com --- drivers/usb/eth/r8152.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 61b8683230..9f7bc7986d 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -1354,9 +1354,8 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, struct usb_interface *iface; struct usb_interface_descriptor *iface_desc; int ep_in_found = 0, ep_out_found = 0; - int i; - struct r8152 *tp; + int i;
/* let's examine the device now */ iface = &dev->config.if_desc[ifnum]; @@ -1399,16 +1398,15 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, if ((iface->ep_desc[i].bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { u8 ep_addr = iface->ep_desc[i].bEndpointAddress; + if ((ep_addr & USB_DIR_IN) && !ep_in_found) { ss->ep_in = ep_addr & USB_ENDPOINT_NUMBER_MASK; ep_in_found = 1; - } else { - if (!ep_out_found) { - ss->ep_out = ep_addr & - USB_ENDPOINT_NUMBER_MASK; - ep_out_found = 1; - } + } else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) { + ss->ep_out = ep_addr & + USB_ENDPOINT_NUMBER_MASK; + ep_out_found = 1; } }
On 5/25/20 9:47 AM, Hayes Wang wrote:
Although I think it never occurs, the code doesn't make sense, because it may allow to assign an IN endpoint to ss->ep_out.
Signed-off-by: Hayes Wang hayeswang@realtek.com
drivers/usb/eth/r8152.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 61b8683230..9f7bc7986d 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -1354,9 +1354,8 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, struct usb_interface *iface; struct usb_interface_descriptor *iface_desc; int ep_in_found = 0, ep_out_found = 0;
- int i;
- struct r8152 *tp;
int i;
/* let's examine the device now */ iface = &dev->config.if_desc[ifnum];
@@ -1399,16 +1398,15 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum, if ((iface->ep_desc[i].bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { u8 ep_addr = iface->ep_desc[i].bEndpointAddress;
if ((ep_addr & USB_DIR_IN) && !ep_in_found) { ss->ep_in = ep_addr & USB_ENDPOINT_NUMBER_MASK; ep_in_found = 1;
} else {if (!ep_out_found) {ss->ep_out = ep_addr &USB_ENDPOINT_NUMBER_MASK;ep_out_found = 1;}
} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {
Sorry, I was wrong in my previous suggestion, the USB_DIR_OUT macro is expanded to 0, so this patch cannot work. 2/2 is already upstream. Do you have a chance to test these patches before sending them ?
Marek Vasut [mailto:marex@denx.de]
Sent: Monday, May 25, 2020 8:03 PM
[...] ep_out_found = 1;
}
} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {Sorry, I was wrong in my previous suggestion, the USB_DIR_OUT macro is expanded to 0, so this patch cannot work. 2/2 is already upstream. Do you have a chance to test these patches before sending them ?
Excuse me. I test v1 only. Do I have to resend v1 for patch #1?
Best Regards, Hayes
On 5/25/20 2:52 PM, Hayes Wang wrote:
Marek Vasut [mailto:marex@denx.de]
Sent: Monday, May 25, 2020 8:03 PM
[...] ep_out_found = 1;
}
} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {Sorry, I was wrong in my previous suggestion, the USB_DIR_OUT macro is expanded to 0, so this patch cannot work. 2/2 is already upstream. Do you have a chance to test these patches before sending them ?
Excuse me. I test v1 only. Do I have to resend v1 for patch #1?
I'll pick V1, no worries.
Marek Vasut [mailto:marex@denx.de]
Sent: Monday, May 25, 2020 9:01 PM
[...]
Excuse me. I test v1 only. Do I have to resend v1 for patch #1?
I'll pick V1, no worries.
Thanks.
Best Regards, Hayes
The PAL_BDC_CR should be PLA_BDC_CR.
Signed-off-by: Hayes Wang hayeswang@realtek.com --- drivers/usb/eth/r8152.c | 8 ++++---- drivers/usb/eth/r8152.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 9f7bc7986d..cef79cab49 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -711,9 +711,9 @@ static void r8152b_enter_oob(struct r8152 *tp)
rtl_rx_vlan_en(tp, false);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PAL_BDC_CR); + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_BDC_CR); ocp_data |= ALDPS_PROXY_MODE; - ocp_write_word(tp, MCU_TYPE_PLA, PAL_BDC_CR, ocp_data); + ocp_write_word(tp, MCU_TYPE_PLA, PLA_BDC_CR, ocp_data);
ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL); ocp_data |= NOW_IS_OOB | DIS_MCU_CLROOB; @@ -844,9 +844,9 @@ static void r8153_enter_oob(struct r8152 *tp)
rtl_rx_vlan_en(tp, false);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PAL_BDC_CR); + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_BDC_CR); ocp_data |= ALDPS_PROXY_MODE; - ocp_write_word(tp, MCU_TYPE_PLA, PAL_BDC_CR, ocp_data); + ocp_write_word(tp, MCU_TYPE_PLA, PLA_BDC_CR, ocp_data);
ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL); ocp_data |= NOW_IS_OOB | DIS_MCU_CLROOB; diff --git a/drivers/usb/eth/r8152.h b/drivers/usb/eth/r8152.h index 09f1c6178b..c7f62b8b3e 100644 --- a/drivers/usb/eth/r8152.h +++ b/drivers/usb/eth/r8152.h @@ -22,7 +22,7 @@ #define PLA_TEREDO_CFG 0xc0bc #define PLA_MAR 0xcd00 #define PLA_BACKUP 0xd000 -#define PAL_BDC_CR 0xd1a0 +#define PLA_BDC_CR 0xd1a0 #define PLA_TEREDO_TIMER 0xd2cc #define PLA_REALWOW_TIMER 0xd2e8 #define PLA_LEDSEL 0xdd90 @@ -225,7 +225,7 @@ #define TEREDO_RS_EVENT_MASK 0x00fe #define OOB_TEREDO_EN 0x0001
-/* PAL_BDC_CR */ +/* PLA_BDC_CR */ #define ALDPS_PROXY_MODE 0x0001
/* PLA_CONFIG34 */
participants (2)
-
Hayes Wang -
Marek Vasut